feat(common): add is_iam_policy_principal_scoped_by_condition helper#208
Conversation
Adds is_iam_policy_principal_scoped_by_condition and the supporting
condition_keys_scoping_principal set to the shared library.
The helper returns true when an IAM policy statement's Condition block
genuinely restricts the caller — covering source-resource (aws:SourceArn),
account/org, VPC, network, and principal-identity keys. Three checks prevent
false negatives:
- Negating operators (containing "not") are rejected, so
StringNotEquals:SourceAccount("X") still reads as "public to everyone
except X".
- ...IfExists operators are rejected; they pass silently when the key is
absent, providing no guaranteed restriction.
- A wildcard value ("*") is rejected; aws:SourceArn:"*" restricts nothing.
This is a superset of is_access_limited_to_an_account_id (account-id scoping
only, presence-based) and is intended for rules that need to avoid false
positives on legitimate AWS service-integration patterns such as
S3/CloudTrail -> SNS event notifications.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d66b4166c3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| not contains(lower(op), "not") | ||
| not endswith(lower(op), "ifexists") | ||
| lower(key) == lower(condition_keys_scoping_principal[_]) | ||
| value != "*" |
There was a problem hiding this comment.
Condition values are not always scalar: Terraform aws_iam_policy_document conversion stores them as []string, and IAM JSON allows arrays. With this comparison, an array such as ["*"] or a network value such as "0.0.0.0/0" for aws:SourceIp is not equal to "*" and therefore makes is_iam_policy_principal_scoped_by_condition return true, even though a wildcard principal remains unrestricted. Rules that use this helper to suppress public-policy findings will miss those policies; normalize arrays and reject wildcard/unrestricted CIDR values before returning scoped.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is interesting: could you give it a thought?
There was a problem hiding this comment.
Fixed the array case — replaced value != "*" with values := as_array(value); not "*" in values, using the existing as_array helper. This catches both the scalar "*" and the array ["*"] form that Terraform's aws_iam_policy_document emits. Added a test for the array wildcard case.
I also handled the wildcard-CIDR suggestion (0.0.0.0/0 for aws:SourceIp)
| not contains(lower(op), "not") | ||
| not endswith(lower(op), "ifexists") | ||
| lower(key) == lower(condition_keys_scoping_principal[_]) | ||
| value != "*" |
There was a problem hiding this comment.
This is interesting: could you give it a thought?
This comment has been minimized.
This comment has been minimized.
…policy_principal_scoped_by_condition Terraform's aws_iam_policy_document emits condition values as arrays, so ["*"] would pass the previous scalar != "*" check. Normalize with as_array() and also reject 0.0.0.0/0 / ::/0 via is_unrestricted. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
7c6bfde to
ced5fb3
Compare
ChouraquiBen
left a comment
ChouraquiBen
left a comment
There was a problem hiding this comment.
LGTM, thanks for going through my comment!
| not endswith(lower(op), "ifexists") | ||
| lower(key) == lower(condition_keys_scoping_principal[_]) | ||
| value != "*" | ||
| values := as_array(value) |
There was a problem hiding this comment.
Remark: ow, nice! That means that you don't have to make two cases depending on whether it is an array or not! 🙏
2 participants
Adds is_iam_policy_principal_scoped_by_condition and the supporting condition_keys_scoping_principal set to the shared library.
The helper returns true when an IAM policy statement's Condition block genuinely restricts the caller — covering source-resource (aws:SourceArn), account/org, VPC, network, and principal-identity keys. Three checks prevent false negatives:
This is a superset of is_access_limited_to_an_account_id (account-id scoping only, presence-based) and is intended for rules that need to avoid false positives on legitimate AWS service-integration patterns such as S3/CloudTrail -> SNS event notifications.
Resources checked in the condition:
aws:SourceArn,aws:SourceOwneraws:SourceAccount,aws:SourceOrgID,aws:SourceOrgPaths,aws:ResourceAccount,aws:VpceAccountaws:SourceVpc,aws:SourceVpce,aws:SourceIp,aws:VpcSourceIpaws:PrincipalArn,aws:PrincipalAccount,aws:PrincipalOrgID,aws:PrincipalOrgPaths