DEV Community

Sergey Shinder
Sergey Shinder

Posted on

An empty list in our module meant every bucket

A pull request gave a reporting service read access to two storage buckets. The plan showed one policy document changing. What we applied gave that service read access to every bucket in the account, and nothing told us for five weeks, until a quarterly access review did.

Our module builds the policy resource from a prefix. The line was an interpolation: the bucket ARN prefix, then the variable, then an asterisk. Callers pass a team prefix, so the reporting team gets their own buckets and nothing else. In this workspace the team prefix variable had never been set, and its default was an empty string, so the rendered resource was the bare ARN root followed by an asterisk. That is the widest expression the service accepts. It means all of them.

There is no reading of an empty string in which the author meant everything. There is also nothing in the language, the provider or the policy engine that treats it as a mistake. An empty string concatenates to nothing, and nothing next to a wildcard is a wildcard.

The review made it worse. A policy document in a plan is an escaped JSON string on a single line, several hundred characters wide, and the change between the old and the new one was the disappearance of eight characters somewhere in the middle. Two of us read it and approved it.

Three things changed. The module has a validation block rejecting a prefix shorter than four characters, which is the fix that would have stopped this on its own. We no longer assemble ARNs by interpolation at all, we build them from an explicit list of names with a for expression, so an empty list produces an empty policy rather than a universal one. And a pipeline step decodes every policy document in a plan and prints the statements as readable rows, then fails the build on any resource that terminates in a bare wildcard unless an exception is recorded next to it.

The habit I would pass on is to ask what each variable means when it is absent, not when it is filled in. We had written the module for the case where somebody sets the value. The case where nobody sets it was the one with unlimited meaning.

– Sergey Shinder

Top comments (0)