Skip to content

Commit aaf9dec

Browse files
VirtueMeclaude
andauthored
fix(iam): resolve local Lambda Ref to static ARN to prevent MalformedPolicyDocument (#746)
When a task state uses `Resource: !Ref localFunction`, two bugs occur: 1. The IAM role's policy resource contains `{ Ref: ParseCSVLambdaFunction }`. CloudFormation resolves a Lambda function Ref to the function *name* (e.g. `my-service-dev-parseCSV`), not its ARN — IAM rejects this with MalformedPolicyDocument. 2. The state machine definition also receives the function name string as its Resource. Step Functions rejects this with InvalidDefinition. In both cases the Ref also introduces a CloudFormation resource dependency that can cause the circular dependency error reported in #470 when the referenced Lambda has an env var pointing back at the state machine. Fix: add `resolveLambdaFunctionName` to `lib/utils/aws.js`. When a Ref points to a local function whose deployed name is known at compile time (via `serverless.service.functions[key].name`), callers emit a static `Fn::Sub` ARN string instead of the Ref. Two call sites are fixed: - `iamStrategies/lambda.js` — IAM policy resource for both the direct lambda ARN path (getFallbackPermissions) and the lambda:invoke SDK integration path (getPermissions) - `compileStateMachines.js` — definition string Fn::Sub params: local Lambda Refs are inlined as static ARNs directly in the definition string, eliminating both the CF dependency and the name-not-ARN error Adds a `circular-dependency` integration fixture reproducing #470. LocalStack confirms: without the fix the IAM role fails with MalformedPolicyDocument and the state machine fails with InvalidDefinition; with the fix the full stack deploys cleanly. Closes #470 Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 31b3ec3 commit aaf9dec

File tree

8 files changed

+215
-12
lines changed

8 files changed

+215
-12
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
'use strict';
2+
3+
module.exports.parseCSV = async () => ({ statusCode: 200 });
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
service: integration-circular-dependency
2+
3+
provider: ${file(../base.yml):provider}
4+
plugins: ${file(../base.yml):plugins}
5+
package: ${file(../base.yml):package}
6+
custom: ${file(../base.yml):custom}
7+
8+
functions:
9+
parseCSV:
10+
handler: handler.parseCSV
11+
12+
stepFunctions:
13+
stateMachines:
14+
circularMachine:
15+
id: CircularMachine
16+
name: integration-circular-${opt:stage, 'test'}
17+
definition:
18+
StartAt: Parse
19+
States:
20+
Parse:
21+
Type: Task
22+
Resource: !Ref ParseCSVLambdaFunction
23+
End: true

fixtures/package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/deploy/stepFunctions/compileStateMachines.js

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@ const aslValidator = require('asl-validator');
55
const BbPromise = require('bluebird');
66
const crypto = require('node:crypto');
77
const schema = require('./compileStateMachines.schema');
8-
const { isIntrinsic, translateLocalFunctionNames, convertToFunctionVersion } = require('../../utils/aws');
8+
const {
9+
isIntrinsic, translateLocalFunctionNames, convertToFunctionVersion, resolveLambdaFunctionName,
10+
} = require('../../utils/aws');
911
const logger = require('../../utils/logger');
1012

1113
function generateSubVariableName(element) {
@@ -155,14 +157,29 @@ module.exports = {
155157
);
156158
}
157159
});
158-
const params = _.fromPairs(functionMappings.map(([k, v]) => [k, f(v)]));
160+
const params = {};
161+
functionMappings.forEach(([k, v]) => {
162+
const translated = f(v);
163+
const functionName = resolveLambdaFunctionName(translated, this);
164+
if (functionName) {
165+
// Inline a static Fn::Sub ARN directly into the definition string.
166+
// Using { Ref: LambdaFunction } resolves to the function *name* at
167+
// CF evaluation time, not its ARN — Step Functions rejects that.
168+
// A static ARN eliminates the CF resource dependency that causes the
169+
// circular dependency error reported in #470.
170+
const staticArn = `arn:\${AWS::Partition}:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${functionName}`;
171+
processedDefinitionString = processedDefinitionString.replace(
172+
new RegExp(`\\$\\{${k}\\}`, 'g'),
173+
staticArn,
174+
);
175+
} else {
176+
params[k] = translated;
177+
}
178+
});
159179

160-
DefinitionString = {
161-
'Fn::Sub': [
162-
processedDefinitionString,
163-
params,
164-
],
165-
};
180+
DefinitionString = _.isEmpty(params)
181+
? { 'Fn::Sub': processedDefinitionString }
182+
: { 'Fn::Sub': [processedDefinitionString, params] };
166183
}
167184
}
168185
}

lib/deploy/stepFunctions/compileStateMachines.test.js

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,6 +1150,55 @@ describe('#compileStateMachines', () => {
11501150
expect(lambda2Param).to.eql({ 'Fn::GetAtt': ['HelloDashworldLambdaFunction', 'Arn'] });
11511151
});
11521152

1153+
it('should inline a static Lambda ARN when Resource is a Ref to a local function with a known name', () => {
1154+
// Reproduces issue #470: `Resource: !Ref parseCSV` compiles to
1155+
// `{ Ref: ParseCSVLambdaFunction }` in the Fn::Sub params map.
1156+
// CloudFormation resolves a Lambda function Ref to the function *name*
1157+
// (e.g. "my-service-dev-parseCSV"), not its ARN — Step Functions then
1158+
// rejects the definition with InvalidDefinition. The fix inlines a static
1159+
// Fn::Sub ARN directly into the definition string when the function name
1160+
// is known at compile time, removing the CF resource reference entirely.
1161+
serverless.service.functions = {
1162+
parseCSV: {
1163+
handler: 'handler.parseCSV',
1164+
name: 'my-service-dev-parseCSV',
1165+
},
1166+
};
1167+
serverless.service.stepFunctions = {
1168+
stateMachines: {
1169+
myStateMachine1: {
1170+
id: 'Test',
1171+
definition: {
1172+
StartAt: 'Parse',
1173+
States: {
1174+
Parse: {
1175+
Type: 'Task',
1176+
Resource: { Ref: 'parseCSV' },
1177+
End: true,
1178+
},
1179+
},
1180+
},
1181+
},
1182+
},
1183+
};
1184+
1185+
serverlessStepFunctions.compileStateMachines();
1186+
const stateMachine = serverlessStepFunctions.serverless.service
1187+
.provider.compiledCloudFormationTemplate.Resources.Test;
1188+
1189+
const definitionString = stateMachine.Properties.DefinitionString;
1190+
const subContent = definitionString['Fn::Sub'];
1191+
const definitionJson = Array.isArray(subContent) ? subContent[0] : subContent;
1192+
const params = Array.isArray(subContent) ? subContent[1] : {};
1193+
1194+
// No Ref to the Lambda function — that would create a CF dependency and
1195+
// resolve to the function name (not ARN) at CF evaluation time
1196+
expect(JSON.stringify(params)).to.not.include('"Ref"');
1197+
1198+
// The resource should be a static Fn::Sub ARN inlined in the definition string
1199+
expect(definitionJson).to.include('arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-service-dev-parseCSV');
1200+
});
1201+
11531202
describe('#useExactVersions', () => {
11541203
beforeEach(() => {
11551204
serverless.service.stepFunctions = {

lib/deploy/stepFunctions/iamStrategies/lambda.js

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
'use strict';
22

33
const _ = require('lodash');
4-
const { isIntrinsic, translateLocalFunctionNames, trimAliasFromLambdaArn } = require('../../../utils/aws');
4+
const {
5+
isIntrinsic, translateLocalFunctionNames, trimAliasFromLambdaArn, resolveLambdaFunctionName,
6+
} = require('../../../utils/aws');
57
const logger = require('../../../utils/logger');
68
const { isJsonPathParameter, isJsonataArgument, getParameterOrArgument } = require('./utils');
79

@@ -59,6 +61,16 @@ function getPermissions(state, { plugin }) {
5961
}
6062

6163
if (_.has(functionName, 'Ref')) {
64+
const resolvedName = resolveLambdaFunctionName(functionName, plugin);
65+
if (resolvedName) {
66+
return [{
67+
action: 'lambda:InvokeFunction',
68+
resource: [
69+
{ 'Fn::Sub': `arn:\${AWS::Partition}:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${resolvedName}` },
70+
{ 'Fn::Sub': `arn:\${AWS::Partition}:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${resolvedName}:*` },
71+
],
72+
}];
73+
}
6274
const functionArn = translateLocalFunctionNames.bind(plugin)(functionName);
6375
return [{
6476
action: 'lambda:InvokeFunction',
@@ -88,6 +100,18 @@ function getPermissions(state, { plugin }) {
88100
function getFallbackPermissions(state, { plugin }) {
89101
if (isIntrinsic(state.Resource) || !!state.Resource.match(/arn:aws(-[a-z]+)*:lambda/)) {
90102
const trimmedArn = trimAliasFromLambdaArn(state.Resource);
103+
104+
const resolvedName = resolveLambdaFunctionName(trimmedArn, plugin);
105+
if (resolvedName) {
106+
return [{
107+
action: 'lambda:InvokeFunction',
108+
resource: [
109+
{ 'Fn::Sub': `arn:\${AWS::Partition}:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${resolvedName}` },
110+
{ 'Fn::Sub': `arn:\${AWS::Partition}:lambda:\${AWS::Region}:\${AWS::AccountId}:function:${resolvedName}:*` },
111+
],
112+
}];
113+
}
114+
91115
const functionArn = translateLocalFunctionNames.bind(plugin)(trimmedArn);
92116
return [{
93117
action: 'lambda:InvokeFunction',

lib/deploy/stepFunctions/iamStrategies/lambda.test.js

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ describe('lambda strategy — getPermissions (lambda:invoke resource type)', ()
151151
expect(result[0].resource).to.deep.include(lambda3);
152152
});
153153

154-
it('should translate local function name Ref to logical resource ID', () => {
154+
it('should translate local function name Ref to logical resource ID when name is not known', () => {
155155
const lambda1 = { Ref: 'hello-world' };
156156
const state = {
157157
Type: 'Task',
@@ -170,6 +170,26 @@ describe('lambda strategy — getPermissions (lambda:invoke resource type)', ()
170170
});
171171
});
172172

173+
it('should use a static Fn::Sub ARN (no Ref) when FunctionName is a Ref to a local function with a known name', () => {
174+
// Same circular dependency risk as #470 but via the lambda:invoke SDK integration
175+
// path: { Ref: LambdaFunction } in the Fn::Sub substitution map still creates a CF
176+
// dependency. When the function name is known at compile time, use a static string.
177+
const state = {
178+
Type: 'Task',
179+
Resource: 'arn:aws:states:::lambda:invoke',
180+
Parameters: {
181+
FunctionName: { Ref: 'hello-world' },
182+
},
183+
};
184+
const plugin = makePlugin({ 'hello-world': { handler: 'hello-world.handler', name: 'my-service-dev-hello-world' } });
185+
const result = getPermissions(state, { plugin });
186+
expect(result[0].action).to.equal('lambda:InvokeFunction');
187+
expect(JSON.stringify(result[0].resource)).to.not.include('"Ref"');
188+
expect(result[0].resource).to.deep.include({
189+
'Fn::Sub': 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-service-dev-hello-world',
190+
});
191+
});
192+
173193
it('should translate local function name Fn::GetAtt to logical resource ID', () => {
174194
const lambda2 = { 'Fn::GetAtt': ['hello-world', 'Arn'] };
175195
const state = {
@@ -288,7 +308,7 @@ describe('lambda strategy — getFallbackPermissions (direct lambda ARN task res
288308
expect(result[0].resource).to.include(baseArn);
289309
});
290310

291-
it('should give lambda:InvokeFunction for an intrinsic function resource (Ref)', () => {
311+
it('should give lambda:InvokeFunction for an intrinsic function resource (Ref to external resource)', () => {
292312
const state = {
293313
Type: 'Task',
294314
Resource: { Ref: 'MyFunction' },
@@ -298,6 +318,41 @@ describe('lambda strategy — getFallbackPermissions (direct lambda ARN task res
298318
expect(result[0].action).to.equal('lambda:InvokeFunction');
299319
});
300320

321+
it('should use a static Fn::Sub ARN (no Ref) when Resource is a Ref to a local function with a known name', () => {
322+
// Reproduces issue #470: using { Ref: localFunction } in the IAM policy resource
323+
// creates a CloudFormation dependency StateMachineRole → Lambda. If the Lambda has
324+
// an env var referencing the state machine ARN, CloudFormation rejects the template
325+
// with a circular dependency error. The fix: resolve the function name at compile
326+
// time and emit a static Fn::Sub string so no CF resource reference is introduced.
327+
const state = {
328+
Type: 'Task',
329+
Resource: { Ref: 'parseCSV' },
330+
};
331+
const plugin = makePlugin({ parseCSV: { handler: 'handler.parseCSV', name: 'my-service-dev-parseCSV' } });
332+
const result = getFallbackPermissions(state, { plugin });
333+
expect(result[0].action).to.equal('lambda:InvokeFunction');
334+
expect(JSON.stringify(result[0].resource)).to.not.include('"Ref"');
335+
expect(result[0].resource).to.deep.include({
336+
'Fn::Sub': 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-service-dev-parseCSV',
337+
});
338+
expect(result[0].resource).to.deep.include({
339+
'Fn::Sub': 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-service-dev-parseCSV:*',
340+
});
341+
});
342+
343+
it('should use a static Fn::Sub ARN when Resource is the CloudFormation logical ID of a local function', () => {
344+
const state = {
345+
Type: 'Task',
346+
Resource: { Ref: 'ParseCSVLambdaFunction' },
347+
};
348+
const plugin = makePlugin({ parseCSV: { handler: 'handler.parseCSV', name: 'my-service-dev-parseCSV' } });
349+
const result = getFallbackPermissions(state, { plugin });
350+
expect(JSON.stringify(result[0].resource)).to.not.include('"Ref"');
351+
expect(result[0].resource).to.deep.include({
352+
'Fn::Sub': 'arn:${AWS::Partition}:lambda:${AWS::Region}:${AWS::AccountId}:function:my-service-dev-parseCSV',
353+
});
354+
});
355+
301356
it('should return [] for a non-lambda, non-intrinsic resource', () => {
302357
const state = {
303358
Type: 'Task',

lib/utils/aws.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,37 @@ function convertToFunctionVersion(value) {
7575
return value;
7676
}
7777

78+
// Resolve a Ref to a local Lambda function to its deployed function name string.
79+
// Returns the name if resolvable, null otherwise.
80+
//
81+
// Using { Ref: LambdaFunctionLogicalId } in an IAM policy resource or state
82+
// machine definition creates a hard CloudFormation dependency and resolves to
83+
// the function *name* (not ARN) at CF evaluation time. IAM rejects non-ARN
84+
// resources; Step Functions rejects non-ARN task resources. Resolving to a
85+
// static name string allows callers to construct a proper Fn::Sub ARN without
86+
// any CF resource reference, eliminating both problems.
87+
function resolveLambdaFunctionName(ref, plugin) {
88+
if (!_.has(ref, 'Ref')) return null;
89+
90+
const refValue = ref.Ref;
91+
const functions = _.get(plugin, 'serverless.service.functions') || {};
92+
93+
// Case 1: Ref is the Serverless function key directly (e.g. 'parseCSV')
94+
let fnKey = _.has(functions, refValue) ? refValue : null;
95+
96+
// Case 2: Ref is the CloudFormation logical ID (e.g. 'ParseCSVLambdaFunction')
97+
if (!fnKey) {
98+
fnKey = Object.keys(functions).find(
99+
(key) => plugin.provider.naming.getLambdaLogicalId(key) === refValue,
100+
) || null;
101+
}
102+
103+
if (!fnKey) return null;
104+
105+
const name = _.get(functions, [fnKey, 'name']);
106+
return typeof name === 'string' ? name : null;
107+
}
108+
78109
// If the resource is a lambda ARN string, trim off the alias
79110
function trimAliasFromLambdaArn(resource) {
80111
if (typeof resource === 'string' && !!resource.match(/^arn:aws(-[a-z]+)*:lambda/)) {
@@ -94,4 +125,5 @@ module.exports = {
94125
translateLocalFunctionNames,
95126
convertToFunctionVersion,
96127
trimAliasFromLambdaArn,
128+
resolveLambdaFunctionName,
97129
};

0 commit comments

Comments
 (0)