diff --git a/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts b/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts index eada6f133..718d8f6ed 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/ambient-global-access.ts @@ -109,7 +109,7 @@ function resolvesToAmbientGlobal( return false; } -function staticMemberName( +export function staticMemberName( node: MemberExpression, scopeAnalysis: ModuleScopeAnalysis, ): string | undefined { diff --git a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts index 7f60d3eea..db6d40b9d 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/extract-connection-ids-from-module-graph.test.ts @@ -8,6 +8,21 @@ import { parseAst } from 'rollup/parseAst'; import { extractConnectionIdsFromModuleGraph } from './extract-connection-ids-from-module-graph'; import { createParsedModuleRecord, type ParsedModuleRecord } from './module-graph'; +import { collectMutatedImportedObjectVariables } from './static-string-resolution'; + +jest.mock('./static-string-resolution', () => { + const actual = jest.requireActual( + './static-string-resolution', + ); + return { + ...actual, + collectMutatedImportedObjectVariables: jest.fn( + actual.collectMutatedImportedObjectVariables, + ), + }; +}); + +const collectMutatedMock = jest.mocked(collectMutatedImportedObjectVariables); const buildRoot = '/project'; const entryId = '/project/src/backend/actions.backend.js'; @@ -746,4 +761,431 @@ describe('Backend Functions - extractConnectionIdsFromModuleGraph', () => { expect(extract([entry, helper])).toEqual(['conn-ts']); }); + + const ACTION_CATALOG_HTTP = '@datadog/action-catalog/http/http'; + const LONG_ALIAS_CHAIN = Array.from({ length: 3000 }, (_, i) => `let a${i + 1} = a${i};`).join( + ' ', + ); + + const createObjectConnectionIdEntry = (statements: string, connectionId = 'CONN.github') => + createRecord( + entryId, + ` + import { request } from '${ACTION_CATALOG_HTTP}'; + + const CONN = { + github: 'conn-github', + gh: { id: 'conn-gh' }, + ready: Promise.resolve({}), + someMethod() { return 1; }, + }; + ${statements} + + export function run() { + return request({ connectionId: ${connectionId}, inputs: {} }); + } + `, + [ACTION_CATALOG_HTTP], + ); + + test.each([ + { description: 'Object.assign', statements: "Object.assign(CONN, { github: 'other' });" }, + { + description: 'Object.defineProperty', + statements: "Object.defineProperty(CONN, 'github', { value: 'other' });", + }, + { + description: 'Object.defineProperties', + statements: "Object.defineProperties(CONN, { github: { value: 'other' } });", + }, + { description: 'Object.setPrototypeOf', statements: 'Object.setPrototypeOf(CONN, null);' }, + { description: 'Reflect.set', statements: "Reflect.set(CONN, 'github', 'other');" }, + { + description: 'Reflect.defineProperty', + statements: "Reflect.defineProperty(CONN, 'github', { value: 'other' });", + }, + { + description: 'Reflect.deleteProperty', + statements: "Reflect.deleteProperty(CONN, 'github');", + }, + { + description: 'Reflect.set with a spread where the receiver would be', + statements: "const extra = []; Reflect.set(CONN, 'github', 'other', ...extra);", + }, + { + description: 'Object.assign called with a computed key', + statements: "Object['assign'](CONN, { github: 'other' });", + }, + { + description: 'Object.assign called with a template-literal computed key', + statements: "Object[`assign`](CONN, { github: 'other' });", + }, + { + description: 'Object.assign called with a const-aliased computed key', + statements: "const method = 'assign'; Object[method](CONN, { github: 'other' });", + }, + { + description: 'Reflect.setPrototypeOf', + statements: 'Reflect.setPrototypeOf(CONN, null);', + }, + { + description: 'Object.assign on one of its members', + statements: "Object.assign(CONN.gh, { id: 'other' });", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through an alias', + statements: "const alias = CONN; alias.github = 'other';", + }, + { + description: 'a write through an alias of one of its members', + statements: "const gh = CONN.gh; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through an alias declared in a function', + statements: "function rename() { const alias = CONN; alias.github = 'other'; }", + }, + { + description: 'a write through a destructured member alias', + statements: "const { gh } = CONN; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through an alias assigned after its declaration', + statements: "let gh; gh = CONN.gh; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'Reflect.set with it as the receiver', + statements: "Reflect.set({}, 'github', 'other', CONN);", + }, + { + description: 'a write through an optional-chaining alias', + statements: "const gh = CONN?.gh; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through an alias with a fallback', + statements: "const gh = CONN.gh || {}; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through a conditional alias', + statements: "const gh = Math.random() > 0.5 ? CONN.gh : null; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through a chained-assignment alias', + statements: "let a, b; a = b = CONN; a.github = 'other';", + }, + { + description: 'a write to a member of a rest copy', + statements: "const { github, ...rest } = CONN; rest.gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through a destructured member of a rest copy', + statements: "const { github, ...rest } = CONN; const { gh } = rest; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write at the end of a long alias chain', + statements: `let a0 = CONN; ${LONG_ALIAS_CHAIN} a3000.github = 'other';`, + }, + { + description: 'a write through a ||= alias', + statements: "let cur = CONN.gh; (cur ||= {}).id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through a ??= alias', + statements: "let cur = CONN.gh; const gh = (cur ??= {}); gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write to a member of a fallback expression', + statements: "(CONN.gh || {}).id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'Object.assign on an alias', + statements: "const alias = CONN; Object.assign(alias, { github: 'other' });", + }, + ])( + 'Should fail closed for an object connection ID mutated by $description', + ({ statements, connectionId }) => { + const entry = createObjectConnectionIdEntry(statements, connectionId); + + expect(() => extract([entry])).toThrow('mutated object binding'); + }, + ); + + test.each([ + { description: 'logged', statements: 'console.log(CONN);' }, + { description: 'passed to Object.keys', statements: 'Object.keys(CONN);' }, + { description: 'passed to JSON.stringify', statements: 'JSON.stringify(CONN);' }, + { description: 'copied by Object.assign', statements: 'Object.assign({}, CONN);' }, + { description: 'aliased without a write', statements: 'const alias = CONN;' }, + { + description: 'read through an unwritten alias', + statements: 'const alias = CONN;', + connectionId: 'alias.github', + }, + { description: 'used to call one of its methods', statements: 'CONN.someMethod();' }, + { + description: 'assigned to an alias that is never written', + statements: 'let gh; gh = CONN.gh;', + }, + { + description: 'destructured into a rest copy that is written', + statements: 'const { gh, ...rest } = CONN; rest.extra = 1;', + }, + { + description: 'passed to a local function named Object', + statements: 'function copy(Object) { Object.assign(CONN, {}); }', + }, + { + description: 'holding a promise whose resolved value is written', + statements: 'async function load() { const result = await CONN.ready; result.x = 1; }', + }, + { + description: 'passed to Reflect.set as the target with another receiver', + statements: "const OTHER = {}; Reflect.set(CONN, 'github', 'other', OTHER);", + }, + { + description: 'guarding a write to another object with &&', + statements: 'const OTHER = {}; const target = CONN.gh && OTHER; target.y = 2;', + }, + ])( + 'Should still resolve an unmutated object connection ID $description', + ({ statements, connectionId }) => { + const entry = createObjectConnectionIdEntry(statements, connectionId); + + expect(extract([entry])).toEqual(['conn-github']); + }, + ); + + const CONNECTIONS_ID = '/project/src/backend/connections.js'; + const CONNECTIONS_CODE = ` + export const CONN = { github: 'conn-github', gh: { id: 'conn-gh' } }; + export const ALIAS = CONN; + export let MUTABLE_ALIAS = CONN.gh; + export const { gh: DESTRUCTURED_ALIAS } = CONN; + export const { github: GITHUB_ID, ...REST } = CONN; + `; + + const extractWithImportedConnections = (statements: string, connectionId = 'CONN.github') => { + const entry = createRecord( + entryId, + ` + import { request } from '${ACTION_CATALOG_HTTP}'; + import { + CONN, + ALIAS, + MUTABLE_ALIAS, + DESTRUCTURED_ALIAS, + REST, + } from './connections.js'; + + ${statements} + + export function run() { + return request({ connectionId: ${connectionId}, inputs: {} }); + } + `, + [ACTION_CATALOG_HTTP, CONNECTIONS_ID], + ); + const connections = createRecord(CONNECTIONS_ID, CONNECTIONS_CODE); + return extract([entry, connections]); + }; + + test.each([ + { description: 'Object.assign', statements: "Object.assign(CONN, { github: 'other' });" }, + { + description: 'Reflect.set with it as the receiver', + statements: "Reflect.set({}, 'github', 'other', CONN);", + }, + { + description: 'a write through an alias its module exports', + statements: "ALIAS.github = 'other';", + }, + { + description: 'a write through a let alias its module exports', + statements: "MUTABLE_ALIAS.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through a destructured alias its module exports', + statements: "DESTRUCTURED_ALIAS.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write to a member of a rest copy', + statements: "const { github, ...rest } = CONN; rest.gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write through a destructured member of a rest copy', + statements: "const { github, ...rest } = CONN; const { gh } = rest; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + { + description: 'a write at the end of a long alias chain', + statements: `let a0 = CONN; ${LONG_ALIAS_CHAIN} a3000.github = 'other';`, + }, + { + description: 'a write through an alias assigned after its declaration', + statements: "let gh; gh = CONN.gh; gh.id = 'other';", + connectionId: 'CONN.gh.id', + }, + ])( + 'Should fail closed for an imported object connection ID mutated by $description', + ({ statements, connectionId }) => { + expect(() => extractWithImportedConnections(statements, connectionId)).toThrow( + 'imported-object-mutation', + ); + }, + ); + + test.each([ + { description: 'read directly', statements: '' }, + { + description: 'read through its exported alias', + statements: '', + connectionId: 'ALIAS.github', + }, + { + description: 'assigned to an alias that is never written', + statements: 'let gh; gh = CONN.gh;', + }, + { description: 'copied by Object.assign', statements: 'Object.assign({}, CONN);' }, + { description: 'copied by a rest export that is written', statements: 'REST.extra = 1;' }, + { + description: 'passed to a local function named Reflect', + statements: "function copy(Reflect) { Reflect.set(CONN, 'github', 'other'); }", + }, + ])( + 'Should still resolve an unmutated imported object connection ID $description', + ({ statements, connectionId }) => { + expect(extractWithImportedConnections(statements, connectionId)).toEqual([ + 'conn-github', + ]); + }, + ); + + test('Should still resolve a local object that shares its name with a namespace re-export written elsewhere', () => { + const otherId = '/project/src/backend/other.js'; + const entry = createRecord( + entryId, + ` + import { CONN as NAMESPACE } from './connections.js'; + + Reflect.set(NAMESPACE, 'github', 'other'); + + export function run() {} + `, + [CONNECTIONS_ID], + ); + const connections = createRecord( + CONNECTIONS_ID, + ` + import { request } from '${ACTION_CATALOG_HTTP}'; + + export * as CONN from './other.js'; + const CONN = { github: 'conn-github' }; + + request({ connectionId: CONN.github, inputs: {} }); + `, + [ACTION_CATALOG_HTTP, otherId], + ); + const other = createRecord(otherId, 'export const value = 1;'); + + expect(extract([entry, connections, other])).toEqual(['conn-github']); + }); + + test.each([ + { description: 'a null __proto__', property: '__proto__: null' }, + { description: 'a shorthand __proto__ property', property: '__proto__' }, + ])( + 'Should still extract the connection ID of call options with $description', + ({ property }) => { + const entry = createRecord( + entryId, + ` + import { request } from '${ACTION_CATALOG_HTTP}'; + + const __proto__ = {}; + + export function run() { + return request({ connectionId: 'conn-a', ${property}, inputs: {} }); + } + `, + [ACTION_CATALOG_HTTP], + ); + + expect(extract([entry])).toEqual(['conn-a']); + }, + ); + + test.each([ + { description: 'a __proto__ key', property: "__proto__: { connectionId: 'hidden' }" }, + { + description: 'a quoted __proto__ key', + property: "'__proto__': { connectionId: 'hidden' }", + }, + { description: 'a toJSON method', property: "toJSON() { return { connectionId: 'b' }; }" }, + { description: 'a then method', property: 'then(resolve) { resolve({}); }' }, + { description: 'a shorthand toJSON property', property: 'toJSON' }, + ])('Should fail closed for action-catalog call options with $description', ({ property }) => { + const entry = createRecord( + entryId, + ` + import { request } from '${ACTION_CATALOG_HTTP}'; + + export function run() { + return request({ connectionId: 'conn-a', ${property}, inputs: {} }); + } + `, + [ACTION_CATALOG_HTTP], + ); + + expect(() => extract([entry])).toThrow('could hide a connectionId'); + }); + + test.each([ + { + description: 'several connectionId call sites', + connectionIds: ['a', 'b', 'c'], + walks: 1, + }, + { description: 'no connectionId call site', connectionIds: [], walks: 0 }, + ])( + 'Should walk the module graph for object writes $walks time(s) per extraction with $description', + ({ connectionIds, walks }) => { + const calls = connectionIds.map( + (id) => `request({ connectionId: CONN.${id}, inputs: {} });`, + ); + const entry = createRecord( + entryId, + ` + import { request } from '${ACTION_CATALOG_HTTP}'; + const CONN = { a: 'conn-a', b: 'conn-b', c: 'conn-c' }; + + export function run() { + request({ inputs: {} }); + ${calls.join('\n')} + } + `, + [ACTION_CATALOG_HTTP], + ); + collectMutatedMock.mockClear(); + + const extracted = extract([entry]); + + const expected = connectionIds.map((id) => `conn-${id}`); + expect(extracted).toEqual(expected); + expect(collectMutatedMock).toHaveBeenCalledTimes(walks); + }, + ); }); 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 704b5db23..f8c991882 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 @@ -2,6 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import type * as eslintScope from 'eslint-scope'; import type { Expression, ObjectExpression, Property, SimpleCallExpression } from 'estree'; import { @@ -10,10 +11,16 @@ import { } from './action-catalog-call-sites'; import { collectActionCatalogImports } from './action-catalog-imports'; import type { ParsedModuleRecord } from './module-graph'; -import { resolveStaticStringValue } from './static-string-resolution'; +import { + collectMutatedImportedObjectVariables, + resolveStaticStringValue, +} from './static-string-resolution'; import { walkModuleGraph } from './walk-module-graph'; const CONNECTION_ID_PROPERTY = 'connectionId'; +// `__proto__` can supply a connectionId from a prototype, and toJSON or then can replace the +// options when they are serialized or awaited. +const OPTION_HOOK_KEYS: ReadonlySet = new Set(['__proto__', 'toJSON', 'then']); type ConnectionIdProperty = Property & { value: Expression }; @@ -27,6 +34,11 @@ export function extractConnectionIdsFromModuleGraph( buildRoot: string, ): string[] { const connectionIds = new Set(); + let mutatedImportedObjectVariables: Set | undefined; + const getMutatedImportedObjectVariables = () => { + mutatedImportedObjectVariables ??= collectMutatedImportedObjectVariables(modules); + return mutatedImportedObjectVariables; + }; // Walk the already-parsed records from this backend entry's build. The // extraction cost is linear in reachable app-local modules, without @@ -36,7 +48,12 @@ export function extractConnectionIdsFromModuleGraph( const scopeAnalysis = analyzeActionCatalogScopes(record.scopeAnalysis, imports); for (const callSite of findActionCatalogCallSites(record.ast, scopeAnalysis, record.id)) { - const connectionId = extractConnectionIdFromActionCall(callSite, modules, record); + const connectionId = extractConnectionIdFromActionCall( + callSite, + modules, + record, + getMutatedImportedObjectVariables, + ); // `''` is a valid declared connectionId, not "none" — only the absence of a // `connectionId` property (`undefined`) means no restriction to record. if (connectionId !== undefined) { @@ -52,6 +69,7 @@ function extractConnectionIdFromActionCall( node: SimpleCallExpression, modules: ReadonlyMap, record: ParsedModuleRecord, + getMutatedImportedObjectVariables: () => ReadonlySet, ): string | undefined { const [firstArg] = node.arguments; if (!firstArg || firstArg.type !== 'ObjectExpression') { @@ -63,7 +81,13 @@ function extractConnectionIdFromActionCall( return undefined; } - const result = resolveStaticStringValue(modules, record.id, connectionIdProperty.value); + const mutatedImportedObjectVariables = getMutatedImportedObjectVariables(); + const result = resolveStaticStringValue( + modules, + record.id, + connectionIdProperty.value, + mutatedImportedObjectVariables, + ); if (result.kind === 'resolved') { return result.value; } @@ -102,6 +126,10 @@ function findConnectionIdProperty( if (property.computed) { throw unsupportedActionCatalogCall(filePath, 'computed object property keys'); } + const key = getStaticPropertyKey(property); + if (key !== undefined && OPTION_HOOK_KEYS.has(key) && !isInertProtoKey(key, property)) { + throw unsupportedActionCatalogCall(filePath, `a "${key}" property`); + } if (isConnectionIdKey(property)) { if (connectionIdProperty) { throw unsupportedActionCatalogCall(filePath, 'multiple connectionId properties'); @@ -131,6 +159,15 @@ function hasExpressionValue(property: Property): property is ConnectionIdPropert ); } +// A shorthand `__proto__` is an own property, and a null prototype supplies nothing. +function isInertProtoKey(key: string, property: Property): boolean { + const { value } = property; + return ( + key === '__proto__' && + (property.shorthand || (value.type === 'Literal' && value.value === null)) + ); +} + function isConnectionIdKey(property: Property): boolean { return getStaticPropertyKey(property) === CONNECTION_ID_PROPERTY; } diff --git a/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts b/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts index d828ac226..7b294352b 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/module-graph.ts @@ -15,12 +15,12 @@ import type { Pattern, Program, SimpleCallExpression, - Super, VariableDeclaration, } from 'estree'; import { importsBackendFunctionPackage, isBackendSourceModule } from '../backend-sources'; +import { staticMemberName } from './ambient-global-access'; import { analyzeModuleScope, getModuleVariable, @@ -28,7 +28,7 @@ import { type ModuleScopeAnalysis, resolveIdentifier, } from './module-scope'; -import { ensureProgram, isStringLiteral } from './type-guards'; +import { ensureProgram, isNodeOfType, isStringLiteral } from './type-guards'; import { walkAst } from './walk-ast'; /** @@ -378,7 +378,7 @@ function collectDeclarationExportBindings( for (const variable of variables) { exportsByName.set(variable.name, { kind: 'unsupported', - reason: 'binding pattern export', + reason: BINDING_PATTERN_EXPORT_REASON, }); } continue; @@ -514,61 +514,70 @@ function collectVariableDeclarationBindings( } } +interface BindingWriteState { + ast: Program; + scopeAnalysis: ModuleScopeAnalysis; + bindings: Map; + // Variables already followed to their alias sources, for writes to the object and to its members. + expandedObjectWrites: Set; + expandedMemberWrites: Set; +} + function markReassignedTopLevelBindings( ast: Program, scopeAnalysis: ModuleScopeAnalysis, bindings: Map, ): void { - walkAst( + const writeState: BindingWriteState = { ast, - { scopeAnalysis, bindings }, - { - AssignmentExpression(node, { state }) { - markAssignedPattern(node.left, state.scopeAnalysis, state.bindings); - }, - UpdateExpression(node, { state }) { - markAssignedPattern(node.argument, state.scopeAnalysis, state.bindings); - }, - UnaryExpression(node, { state }) { - if (node.operator === 'delete') { - markAssignedPattern(node.argument, state.scopeAnalysis, state.bindings); - } - }, - ForInStatement(node, { state }) { - markForIterationTarget(node.left, state.scopeAnalysis, state.bindings); - }, - ForOfStatement(node, { state }) { - markForIterationTarget(node.left, state.scopeAnalysis, state.bindings); - }, + scopeAnalysis, + bindings, + expandedObjectWrites: new Set(), + expandedMemberWrites: new Set(), + }; + walkAst(ast, writeState, { + AssignmentExpression(node, { state }) { + markAssignedPattern(node.left, state); }, - ); + UpdateExpression(node, { state }) { + markAssignedPattern(node.argument, state); + }, + UnaryExpression(node, { state }) { + if (node.operator === 'delete') { + markAssignedPattern(node.argument, state); + } + }, + CallExpression(node, { state }) { + for (const target of getMutatedCallTargets(node, state.scopeAnalysis)) { + markMutatedObject(target, state); + } + }, + ForInStatement(node, { state }) { + markForIterationTarget(node.left, state); + }, + ForOfStatement(node, { state }) { + markForIterationTarget(node.left, state); + }, + }); } function markForIterationTarget( left: Pattern | VariableDeclaration, - scopeAnalysis: ModuleScopeAnalysis, - bindings: Map, + state: BindingWriteState, ): void { if (left.type !== 'VariableDeclaration') { - markAssignedPattern(left, scopeAnalysis, bindings); + markAssignedPattern(left, state); } } -function markAssignedPattern( - pattern: Pattern | Expression, - scopeAnalysis: ModuleScopeAnalysis, - bindings: Map, -): void { +function markAssignedPattern(pattern: Pattern | Expression, state: BindingWriteState): void { if (pattern.type === 'Identifier') { - markAssignedVariable(pattern, scopeAnalysis, bindings, 'reassigned binding'); + markAssignedVariable(pattern, state.scopeAnalysis, state.bindings, 'reassigned binding'); return; } if (pattern.type === 'MemberExpression') { - const root = getMemberExpressionRoot(pattern); - if (root) { - markAssignedVariable(root, scopeAnalysis, bindings, 'mutated object binding'); - } + markMutatedObject(pattern.object, state); return; } @@ -576,8 +585,7 @@ function markAssignedPattern( for (const property of pattern.properties) { markAssignedPattern( property.type === 'RestElement' ? property.argument : property.value, - scopeAnalysis, - bindings, + state, ); } return; @@ -586,19 +594,188 @@ function markAssignedPattern( if (pattern.type === 'ArrayPattern') { for (const element of pattern.elements) { if (element) { - markAssignedPattern(element, scopeAnalysis, bindings); + markAssignedPattern(element, state); } } return; } if (pattern.type === 'RestElement') { - markAssignedPattern(pattern.argument, scopeAnalysis, bindings); + markAssignedPattern(pattern.argument, state); return; } if (pattern.type === 'AssignmentPattern') { - markAssignedPattern(pattern.left, scopeAnalysis, bindings); + markAssignedPattern(pattern.left, state); + } +} + +export const BINDING_PATTERN_EXPORT_REASON = 'binding pattern export'; + +const OBJECT_MUTATORS = new Map>([ + ['Object', new Set(['assign', 'defineProperty', 'defineProperties', 'setPrototypeOf'])], + ['Reflect', new Set(['set', 'defineProperty', 'deleteProperty', 'setPrototypeOf'])], +]); + +/** Returns the objects that a call such as `Object.assign(target, ...)` writes to. */ +export function getMutatedCallTargets( + call: SimpleCallExpression, + scopeAnalysis: ModuleScopeAnalysis, +): Expression[] { + const { callee } = call; + if (callee.type !== 'MemberExpression' || callee.object.type !== 'Identifier') { + return []; + } + const methodName = staticMemberName(callee, scopeAnalysis); + if ( + methodName === undefined || + !OBJECT_MUTATORS.get(callee.object.name)?.has(methodName) || + resolveIdentifier(callee.object, scopeAnalysis) + ) { + return []; + } + // `Reflect.set` writes to its receiver, the fourth argument, when one is given. With a spread + // among the first four, both the first and fourth are marked, but not one shifted into place. + const [target, , , receiver] = call.arguments; + const isReflectSet = callee.object.name === 'Reflect' && methodName === 'set'; + const hasSpread = call.arguments.slice(0, 4).some((arg) => arg.type === 'SpreadElement'); + let targets = [target]; + if (isReflectSet) { + targets = hasSpread ? [target, receiver] : [receiver ?? target]; + } + return targets.filter( + (argument): argument is Expression => + argument !== undefined && argument.type !== 'SpreadElement', + ); +} + +/** A variable whose object an expression may evaluate to, or reach a member of. */ +export interface ObjectRoot { + identifier: Identifier; + isMember: boolean; +} + +/** Lists the roots of expressions such as `CONN`, `CONN.gh`, `CONN?.gh`, `CONN.gh || {}` or `a = CONN`. */ +export function getObjectRoots(node: unknown, isMember = false): ObjectRoot[] { + if (isNodeOfType(node, 'Identifier')) { + return [{ identifier: node, isMember }]; + } + if (isNodeOfType(node, 'MemberExpression')) { + return getObjectRoots(node.object, true); + } + if (isNodeOfType(node, 'ChainExpression')) { + return getObjectRoots(node.expression, isMember); + } + if (isNodeOfType(node, 'AssignmentExpression')) { + // `a ||= b` and `a ??= b` yield `a` when it already holds an object. + const rightRoots = getObjectRoots(node.right, isMember); + return node.operator === '||=' || node.operator === '??=' + ? [...getObjectRoots(node.left, isMember), ...rightRoots] + : rightRoots; + } + if (isNodeOfType(node, 'SequenceExpression')) { + const lastExpression = node.expressions.at(-1); + return getObjectRoots(lastExpression, isMember); + } + if (isNodeOfType(node, 'LogicalExpression')) { + // `a && b` only yields `a` when it is falsy, which no object is. + const rightRoots = getObjectRoots(node.right, isMember); + return node.operator === '&&' + ? rightRoots + : [...getObjectRoots(node.left, isMember), ...rightRoots]; + } + if (isNodeOfType(node, 'ConditionalExpression')) { + return [ + ...getObjectRoots(node.consequent, isMember), + ...getObjectRoots(node.alternate, isMember), + ]; + } + return []; +} + +type PatternTargetKind = 'member' | 'rest'; + +const patternTargetsByProgram = new WeakMap>(); + +// Identifiers bound by destructuring: a rest element gets a new object, any other target a member. +function getPatternTargets(ast: Program): Map { + let targets = patternTargetsByProgram.get(ast); + if (!targets) { + targets = new Map(); + walkAst(ast, targets, { + ObjectPattern(node, { state }) { + for (const property of node.properties) { + if (property.type === 'Property') { + markMemberTarget(property.value, state); + } + } + }, + ArrayPattern(node, { state }) { + for (const element of node.elements) { + markMemberTarget(element, state); + } + }, + RestElement(node, { state }) { + state.set(node.argument, 'rest'); + }, + }); + patternTargetsByProgram.set(ast, targets); + } + return targets; +} + +function markMemberTarget(node: Pattern | null, targets: Map): void { + const target = node?.type === 'AssignmentPattern' ? node.left : node; + if (target?.type === 'Identifier' && !targets.has(target)) { + targets.set(target, 'member'); + } +} + +/** + * Lists the roots `variable`, declared in `ast`, was assigned from. A rest element copies only + * its source's top-level properties, so it is followed only for a write to one of its members. + */ +export function getAliasSourceRoots( + variable: eslintScope.Variable, + ast: Program, + isMember: boolean, +): ObjectRoot[] { + const patternTargets = getPatternTargets(ast); + const roots: ObjectRoot[] = []; + for (const reference of variable.references) { + const targetKind = patternTargets.get(reference.identifier); + if (!reference.writeExpr || (!isMember && targetKind === 'rest')) { + continue; + } + const isSourceMember = isMember || targetKind === 'member'; + for (const root of getObjectRoots(reference.writeExpr)) { + roots.push({ identifier: root.identifier, isMember: isSourceMember || root.isMember }); + } + } + return roots; +} + +// A write through an alias such as `const gh = CONN.gh` or `gh = CONN.gh` also changes the object it aliases. +function markMutatedObject(writtenObject: unknown, state: BindingWriteState): void { + const pending = getObjectRoots(writtenObject); + for (let root = pending.pop(); root; root = pending.pop()) { + markAssignedVariable( + root.identifier, + state.scopeAnalysis, + state.bindings, + 'mutated object binding', + ); + + const variable = resolveIdentifier(root.identifier, state.scopeAnalysis); + const expanded = root.isMember ? state.expandedMemberWrites : state.expandedObjectWrites; + if (!variable || expanded.has(variable)) { + continue; + } + expanded.add(variable); + const sourceRoots = getAliasSourceRoots(variable, state.ast, root.isMember); + for (const sourceRoot of sourceRoots) { + pending.push(sourceRoot); + } } } @@ -620,16 +797,6 @@ function markAssignedVariable( bindings.set(variable, { kind: 'unsupported', reason }); } -function getMemberExpressionRoot(node: Expression | Super): Identifier | undefined { - if (node.type === 'Identifier') { - return node; - } - if (node.type === 'MemberExpression') { - return getMemberExpressionRoot(node.object); - } - return undefined; -} - function getModuleExportName(node: ModuleExportName): string { if (node.type === 'Identifier') { return node.name; 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 6374f3932..4bb565abb 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 @@ -14,11 +14,17 @@ import type { Super, TemplateLiteral, VariableDeclaration, - VariableDeclarator, } from 'estree'; -import type { ParsedModuleRecord } from './module-graph'; -import { isImportVariable, resolveIdentifier } from './module-scope'; +import { + BINDING_PATTERN_EXPORT_REASON, + getAliasSourceRoots, + getMutatedCallTargets, + getObjectRoots, + type ObjectRoot, + type ParsedModuleRecord, +} from './module-graph'; +import { getModuleVariable, isImportVariable, resolveIdentifier } from './module-scope'; import { resolveStaticDefinitionForIdentifier, type UnsupportedStaticDefinition, @@ -171,6 +177,19 @@ interface ImportedObjectMutationState { modules: ReadonlyMap; mutatedVariables: Set; record: ParsedModuleRecord; + // The write kinds each variable was already followed to its alias sources for. + expandedAliases: Map>; +} + +interface PendingImportedAlias extends ObjectRoot { + record: ParsedModuleRecord; + isPastImport: boolean; +} + +interface ResolvedImportedAlias { + record: ParsedModuleRecord; + variable: eslintScope.Variable; + isPastImport: boolean; } interface ResolvedStaticExpression { @@ -201,11 +220,14 @@ export function resolveStaticStringValue( modules: ReadonlyMap, moduleId: string, expression: Expression, + mutatedImportedObjectVariables: ReadonlySet = collectMutatedImportedObjectVariables( + modules, + ), ): StaticStringValueResolution { return resolveExpression( { modules, - mutatedImportedObjectVariables: collectMutatedImportedObjectVariables(modules), + mutatedImportedObjectVariables, seenVariables: new Set(), }, moduleId, @@ -456,15 +478,16 @@ function hasExpressionValue(property: Property): property is Property & { value: ); } -function collectMutatedImportedObjectVariables( +export function collectMutatedImportedObjectVariables( modules: ReadonlyMap, ): Set { const mutatedVariables = new Set(); + const expandedAliases = new Map>(); for (const record of modules.values()) { walkAst( record.ast, - { modules, mutatedVariables, record }, + { modules, mutatedVariables, record, expandedAliases }, { AssignmentExpression(node, { state }) { markMutatedImportedPattern(node.left, state); @@ -477,6 +500,11 @@ function collectMutatedImportedObjectVariables( markMutatedImportedPattern(node.argument, state); } }, + CallExpression(node, { state }) { + for (const target of getMutatedCallTargets(node, state.record.scopeAnalysis)) { + markMutatedImportedObject(target, state); + } + }, ForInStatement(node, { state }) { markMutatedImportedForIterationTarget(node.left, state); }, @@ -504,15 +532,16 @@ function markMutatedImportedPattern( state: ImportedObjectMutationState, ): void { if (pattern.type === 'Identifier') { - markMutatedImportedIdentifier(pattern, state); + // Rebinding a local name leaves the object it held unchanged; only an import's own binding counts. + const variable = resolveIdentifier(pattern, state.record.scopeAnalysis); + if (variable && isImportVariable(variable)) { + markMutatedImportedObject(pattern, state); + } return; } if (pattern.type === 'MemberExpression') { - const root = getMemberExpressionRoot(pattern); - if (root) { - markMutatedImportedIdentifier(root, state); - } + markMutatedImportedObject(pattern.object, state); return; } @@ -545,77 +574,82 @@ function markMutatedImportedPattern( } } -function markMutatedImportedIdentifier( - identifier: Identifier, +// Marks the imported objects `writtenObject` may alias. Past an import, the definition module's +// own aliases of the object (e.g. `export const ALIAS = CONN`) are marked too. +function markMutatedImportedObject( + writtenObject: unknown, state: ImportedObjectMutationState, ): void { - const variable = resolveImportedObjectAlias(state, identifier, new Set()); - if (variable) { - state.mutatedVariables.add(variable); + const pending: PendingImportedAlias[] = getObjectRoots(writtenObject).map((root) => ({ + ...root, + record: state.record, + isPastImport: false, + })); + for (let alias = pending.pop(); alias; alias = pending.pop()) { + const resolved = resolveImportedAlias(state, alias); + if (!resolved) { + continue; + } + if (resolved.isPastImport) { + state.mutatedVariables.add(resolved.variable); + } + const expanded = state.expandedAliases.get(resolved.variable) ?? new Set(); + const writeKind = `${alias.isMember}:${resolved.isPastImport}`; + if (expanded.has(writeKind)) { + continue; + } + expanded.add(writeKind); + state.expandedAliases.set(resolved.variable, expanded); + for (const root of getAliasSourceRoots( + resolved.variable, + resolved.record.ast, + alias.isMember, + )) { + pending.push({ ...root, record: resolved.record, isPastImport: resolved.isPastImport }); + } } } -function resolveImportedObjectAlias( +function resolveImportedAlias( state: ImportedObjectMutationState, - identifier: Identifier, - seenVariables: Set, -): eslintScope.Variable | undefined { - const variable = resolveIdentifier(identifier, state.record.scopeAnalysis); - if (!variable || seenVariables.has(variable)) { + { record, identifier, isPastImport }: PendingImportedAlias, +): ResolvedImportedAlias | undefined { + const variable = resolveIdentifier(identifier, record.scopeAnalysis); + if (!variable) { return undefined; } - seenVariables.add(variable); - - if (isImportVariable(variable)) { - const definition = resolveStaticDefinitionForIdentifier( - state.modules, - state.record.id, - identifier, - ); - return definition.kind === 'local' ? definition.variable : undefined; + if (!isImportVariable(variable)) { + return { record, variable, isPastImport }; } - const initializer = getVariableInitializer(variable); - if (!initializer) { - return undefined; - } - - const root = getImportedObjectAliasRoot(initializer); - if (!root) { - return undefined; - } - - return resolveImportedObjectAlias(state, root, seenVariables); + const definition = resolveStaticDefinitionForIdentifier(state.modules, record.id, identifier); + const definitionRecord = state.modules.get(definition.moduleId); + const definitionVariable = + definition.kind === 'local' + ? definition.variable + : getAliasVariable(definition, definitionRecord); + return definitionRecord && definitionVariable + ? { record: definitionRecord, variable: definitionVariable, isPastImport: true } + : undefined; } -function getVariableInitializer(variable: eslintScope.Variable): Expression | undefined { - for (const definition of variable.defs) { - const node = definition.node as VariableDeclarator; - if (node.type === 'VariableDeclarator' && node.init) { - return node.init; - } - } - - return undefined; -} - -function getImportedObjectAliasRoot(rawExpression: Expression): Identifier | undefined { - const expression = unwrapStaticExpression(rawExpression); - if (expression.type === 'Identifier') { - return expression; - } - if (expression.type === 'MemberExpression') { - return getMemberExpressionRoot(expression); +// An exported top-level `let`, `var` or destructured declaration is not a static definition, but +// a write through it still reaches the object it was assigned from. +function getAliasVariable( + definition: UnsupportedStaticDefinition, + record: ParsedModuleRecord | undefined, +): eslintScope.Variable | undefined { + if (!record) { + return undefined; } - return undefined; -} - -function getMemberExpressionRoot(node: Expression | Super): Identifier | undefined { - if (node.type === 'Identifier') { - return node; + if (definition.reason === 'mutable-binding' || definition.reason === 'unsupported-binding') { + return getModuleVariable(definition.variableName, record.scopeAnalysis); } - if (node.type === 'MemberExpression') { - return getMemberExpressionRoot(node.object); + if ( + definition.reason === 'unsupported-export' && + definition.exportReason === BINDING_PATTERN_EXPORT_REASON + ) { + return getModuleVariable(definition.exportName, record.scopeAnalysis); } return undefined; } 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 fb693d135..5258b61b0 100644 --- a/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts +++ b/packages/plugins/apps/src/backend/ast-parsing/type-guards.ts @@ -2,7 +2,14 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import type { BaseNode, Program, SimpleLiteral, TemplateLiteral, VariableDeclarator } from 'estree'; +import type { + BaseNode, + Node, + Program, + SimpleLiteral, + TemplateLiteral, + VariableDeclarator, +} from 'estree'; export type StringLiteral = SimpleLiteral & { value: string }; @@ -43,6 +50,14 @@ export function isVariableDeclaratorNode(node: unknown): node is VariableDeclara return hasNodeType(node, 'VariableDeclarator'); } +// Narrows nodes typed against another copy of `estree`, such as eslint-scope's, by their type. +export function isNodeOfType( + node: unknown, + type: T, +): node is Extract { + return hasNodeType(node, type); +} + function isNoSubstitutionTemplateLiteral(node: unknown): node is TemplateLiteral { if (!hasNodeType(node, 'TemplateLiteral') || !('expressions' in node) || !('quasis' in node)) { return false;