Add condition for sts:AssumeRole of the lambda principal to be from within the correct source account by gligorkot · Pull Request #13125

Add condition for sts:AssumeRole of the lambda principal to be from within the correct source account by gligorkot · Pull Request #13125 · serverless/serverless

gligorkot commented
Sep 25, 2025
•

edited by coderabbitai bot

Loading

This PR implements the change to update the default IAM Lambda execution role, by adding a condition that the assumer of the role comes from within the same AWS Account that the role is deployed in. From a security point of view this seems like a good idea for this to be the default option.

Closes: #13124

Summary by CodeRabbit

Bug Fixes
- Role assumption now enforces account-level validation so requests must originate from the same AWS account.
- When adding additional invocation sources, existing account-restriction conditions are preserved to maintain prior access controls.
Tests
- Added unit tests to verify preservation of account-level conditions when updating role principals.

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

I have read the CLA Document and I hereby sign the CLA

…ithin the correct source account

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ab140eed-8c1b-4516-af56-4b04c82a0d1b

📥 Commits

Reviewing files that changed from the base of the PR and between bd977e1 and 0fd43c0.

📒 Files selected for processing (1)

packages/serverless/test/unit/lib/plugins/aws/package/lib/roles-per-function-permissions.test.js

📝 Walkthrough

Walkthrough

A Condition was added to the IAM role trust policy requiring aws:SourceAccount to equal the deployment AWS::AccountId. Two unit tests were added to verify preservation of an existing aws:SourceAccount Condition when adding CloudFront and Scheduler principals.

Changes

Cohort / File(s)	Summary
IAM Role Trust Policy `packages/serverless/lib/plugins/aws/package/lib/iam-role-lambda-execution-template.json`	Added a `StringEquals` Condition on `aws:SourceAccount` with `Ref: AWS::AccountId` to the `AssumeRolePolicyDocument` trust statement for Lambda execution.
Unit tests for per-function role principals `packages/serverless/test/unit/lib/plugins/aws/package/lib/roles-per-function-permissions.test.js`	Added two tests ensuring an existing `aws:SourceAccount` Condition is preserved when adding `edgelambda.amazonaws.com` (CloudFront viewer-request) and `scheduler.amazonaws.com` (schedule) principals alongside `lambda.amazonaws.com`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 I nibbled code beneath the moon so bright,

I added a check to keep account details tight.
Principals may knock but must show their name,
One small Condition secures the role's domain.
Hoppity-hop — safe by starlight! 🥕✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: adding a condition to the AssumeRole statement requiring the source account to match the deployed account.
Linked Issues check	✅ Passed	The PR fully implements the requirement from issue `#13124` by adding an aws:SourceAccount condition to the IAM role's AssumeRolePolicyDocument and includes tests verifying the condition is preserved.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the source account condition requirement; the template modification and corresponding unit tests are properly scoped to the objective.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)

packages/serverless/lib/plugins/aws/package/lib/iam-role-lambda-execution-template.json (1)

13-17: Add regression tests to lock in the new trust-policy guard.

Good security hardening, but there’s no unit assertion proving this Condition survives downstream trust-policy mutations (notably in packages/serverless/lib/plugins/aws/package/lib/roles-per-function-permissions.js, Line 336-377). Please add/extend tests to assert aws:SourceAccount is present after role construction paths.

Suggested test assertion shape

 AssumeRolePolicyDocument: {
   Statement: [
     {
       Effect: 'Allow',
       Principal: { Service: 'lambda.amazonaws.com' },
       Action: 'sts:AssumeRole',
+      Condition: {
+        StringEquals: {
+          'aws:SourceAccount': { Ref: 'AWS::AccountId' },
+        },
+      },
     },
   ],
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/serverless/lib/plugins/aws/package/lib/iam-role-lambda-execution-template.json`
around lines 13 - 17, Add unit tests that assert the trust-policy Condition
"StringEquals":{"aws:SourceAccount":{ "Ref":"AWS::AccountId"}} is retained after
role construction and any downstream mutations; specifically extend or add tests
around the role construction pipeline that exercises the code in
roles-per-function-permissions.js (the logic around Line 336-377) and the
template defined in iam-role-lambda-execution-template.json so the final
generated role/trust policy contains aws:SourceAccount in the Condition. In the
test, construct the role via the same function/class used in packaging (invoke
the role creation path that consumes iam-role-lambda-execution-template.json and
passes through roles-per-function-permissions.js), then assert the resulting
CloudFormation trust policy JSON includes the aws:SourceAccount Condition on the
principal/sts:AssumeRole statements; fail the test if that key is missing or
altered.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In
`@packages/serverless/lib/plugins/aws/package/lib/iam-role-lambda-execution-template.json`:
- Around line 13-17: Add unit tests that assert the trust-policy Condition
"StringEquals":{"aws:SourceAccount":{ "Ref":"AWS::AccountId"}} is retained after
role construction and any downstream mutations; specifically extend or add tests
around the role construction pipeline that exercises the code in
roles-per-function-permissions.js (the logic around Line 336-377) and the
template defined in iam-role-lambda-execution-template.json so the final
generated role/trust policy contains aws:SourceAccount in the Condition. In the
test, construct the role via the same function/class used in packaging (invoke
the role creation path that consumes iam-role-lambda-execution-template.json and
passes through roles-per-function-permissions.js), then assert the resulting
CloudFormation trust policy JSON includes the aws:SourceAccount Condition on the
principal/sts:AssumeRole statements; fail the test if that key is missing or
altered.

ℹ️ Review info