fleece icon indicating copy to clipboard operation
fleece copied to clipboard

Bug: AWS accounts with leading zero are treated as octal

Open njgarratt opened this issue 6 years ago • 5 comments

Hello,

An interesting bug was revealed in fleece today where an aws account number had a leading zero. This number was treated as octal and converted to decimal automatically which then obviously resulted in the error:

Could not fetch AWS Account credentials.
Status: 404
Reason: AWS account xxxxxxxx does not exist.

The correct behaviour would be to not perform this conversion on the input.

njgarratt avatar Mar 25 '20 11:03 njgarratt

Thanks for finding that. I'm seeing a few places that could be occurring, but not immediately anywhere we are converting it. I think we might need to just force certain args to be a string. What specifically were you going to get that error?

gifflen avatar Mar 25 '20 18:03 gifflen

It actually looks like we are casting inputs via the args parser as strings. Was that account coming in via an environments.yml file or a config file?

gifflen avatar Mar 25 '20 18:03 gifflen

Hey Bruce, this appears to be a bug in PyYAML, which is used by run.py. Switching to ruamel.yaml will fix the octal issue, but then there’s the problem of leading zeroes being swallowed by the yaml parser. To fix the latter, fleece needs to force config files to define account numbers as strings. That would be a breaking change and a ton of config files would need to be updated.

On Wed, 25 Mar 2020 at 19:47, Bruce Stringer [email protected] wrote:

It actually looks like we are casting inputs via the args parser as strings. Was that account coming in via an environments.yml file or a config file?

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rackerlabs/fleece/issues/109#issuecomment-604019780, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADRMCTX4GUYZ5ZTTRI27L3RJJGVRANCNFSM4LTMLLUQ .

larsbutler avatar Mar 25 '20 22:03 larsbutler

@njgarratt - Thanks for reporting this! I don't believe this is a bug in PyYAML per se as it seems from this discussion by the author of ruamel.yaml that it is in fact part of how YAML is supposed to parse things for the pre-2009 YAML 1.1 specification. It seems this is a decidedly funky way YAML works. :)

We can certainly investigate trying to return a helpful message if the account number isn't parsed as a string as expected (this should be somewhat easy to determine), and I'll definitely poke at seeing if we can use some of the options suggested by Anthon in his comment above such as yaml = YAML(typ='base') to force all scalars to string since we don't handle any other types in this document and I don't expect that to change.

However, in the meantime, I think your specific issue can be resolved by simply wrapping you account number in quotes like in the example from the readme:

# cat environments.yml
environments:
  - name: development
    account: '123456789012'
  - name: staging
    account: '123456789012'
    rs_username_var: MY_RS_USERNAME
    rs_apikey_var: MY_RS_APIKEY
  - name: testing
    account: '123456789012'
  - name: production
    account: '123456789012'
    role: LambdaDeployRole

This will tell PyYAML that the scalar is a string instead of it interpreting it as an octal.

ryandub avatar Mar 31 '20 16:03 ryandub

Docs have been updated to be explicit about quoting accounts.

gifflen avatar Mar 31 '20 21:03 gifflen