aws-cloudformation / aws-cloudformation/aws-cloudformation-samples

Boolean values converted to strings not handled properly

Open
#43 2 comments 3 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
147
Forks
59
PR merge metrics
No merged PRs in 30d

Description

While developing a [new hook type](https://www.openpolicyagent.org/docs/latest/aws-cloudformation-hooks/) to allow Open Policy Agent (OPA) to make decisions on infrastructure deployments via CloudFormation hooks, one of the challenges we faced was how boolean values in the YAML/JSON templates are converted to strings before they're presented to the hook. This behavior is surprising, and leads to seemingly correct policies that should **deny** a deployment instead **allowing** resource creation or modifications, as checking for "truthiness" will always return true (as both strings "true" and "false" are truthy).

While comparing our policies to the ones in this repository, I was surprised to find that these sample policies don't seem to take this into account.

While the main issue here is that this conversion is being done in the first place (and if there's an issue to track that, I'd love to know where I can find it!), the examples should at least be updated to take this into account, as they currently will allow resources to be created that aren't compliant with the hook policy deployed. To provide an example, the Python sample to [block public access to S3 buckets](https://github.com/aws-cloudformation/aws-cloudformation-samples/tree/main/hooks/python-hooks/s3-block-public-access) will happily accept a template like the one provided below:

```yaml
Resources:
S3Bucket:
Type: 'AWS::S3::Bucket'
Properties:
BucketName: !Sub 'mybucket-${AWS::Region}-${AWS::AccountId}'
PublicAccessBlockConfiguration:
BlockPublicAcls: false
BlockPublicPolicy: false
IgnorePublicAcls: false
RestrictPublicBuckets: false
```
The corresponding handler code that checks for these values will pass as long as the values are set, even when set to `false`, as the converted `"false"` is truthy:

https://github.com/aws-cloudformation/aws-cloudformation-samples/blob/a3f2e64e66a3e33aa910cab9b34dcb4598bf5f92/hooks/python-hooks/s3-block-public-access/src/awssamples_s3blockpublicaccess_hook/handlers.py#L58-L64

I'm sure there are more examples like this, and while they can be fixed, it would be preferrable if the core issue—i.e. the conversion taking place—was addressed.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.