Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quotation Marks in Environment Variables #650

Closed
ericfrederich opened this issue Mar 22, 2024 · 2 comments · Fixed by #651
Closed

Quotation Marks in Environment Variables #650

ericfrederich opened this issue Mar 22, 2024 · 2 comments · Fixed by #651
Assignees
Labels
bug_with_workaround Something isn't working but there is a workaround

Comments

@ericfrederich
Copy link

Describe the bug

Environment variables are not being exported as you'd imagine. The quotation marks from the example in the README are being exported. Perhaps some 3rd party tool like terraform is ignoring/stripping them? They do not belong.

If this behavior is desired, I'd recommend simply changing the example to not include quotes.

SIDE NOTE: I believe this example promotes bad practice anyway. The .pre-commit-config.yaml is gets checked into Git. Secrets like these should never be put in Git, so this config should not contain secrets. Instead, if this functionality is needed, one should configure a proper profile and set the AWS_PROFILE environment variable instead.

I noticed this when trying to implement the same feature in Python for #648, but now I need clarity on this ;-)

How can we reproduce it?

This is evident if you do the use the example from the README...

- id: terraform_fmt
  args:
    - --env-vars=AWS_DEFAULT_REGION="us-west-2"
    - --env-vars=AWS_ACCESS_KEY_ID="anaccesskey"
    - --env-vars=AWS_SECRET_ACCESS_KEY="asecretkey"

... and then modify your terraform_fmt.sh to call the following immediately after common::parse_and_export_env_vars to see what's actually being set in the environment:

env | grep AWS_ >> /tmp/env_vars
python3 -c "import os; print({k: v for k, v in os.environ.items() if k.startswith('AWS_')})" >> /tmp/env_vars

You'll see in /tmp/env_vars that the quotes came along. This is incorrect.

AWS_DEFAULT_REGION="us-west-2"
AWS_SECRET_ACCESS_KEY="asecretkey"
AWS_ACCESS_KEY_ID="anaccesskey"
{'AWS_DEFAULT_REGION': '"us-west-2"', 'AWS_SECRET_ACCESS_KEY': '"asecretkey"', 'AWS_ACCESS_KEY_ID': '"anaccesskey"'}

Environment information

  • OS: Win11 with Ubuntu 22.04 on WSL2
  • uname -a and/or systeminfo | Select-String "^OS" output:
Linux MYPCNAME 5.15.146.1-microsoft-standard-WSL2 #1 SMP Thu Jan 11 04:09:03 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux
  • Tools availability and versions:
GNU bash, version 5.1.16(1)-release (x86_64-pc-linux-gnu)
pre-commit 3.6.2
Terraform v1.5.6
python SKIPPED
Python 3.10.12
checkov SKIPPED
infracost SKIPPED
terraform-docs SKIPPED
terragrunt SKIPPED
terrascan SKIPPED
tflint SKIPPED
tfsec SKIPPED
trivy SKIPPED
tfupdate SKIPPED
hcledit SKIPPED
  • .pre-commit-config.yaml:
file content
repos:
- repo: /home/eric/src/pre-commit-terraform
  rev: some_local_ref
  hooks:
  - id: terraform_fmt
    args:
      - --env-vars=AWS_DEFAULT_REGION="us-west-2"
      - --env-vars=AWS_ACCESS_KEY_ID="anaccesskey"
      - --env-vars=AWS_SECRET_ACCESS_KEY="asecretkey"
@yermulnik
Copy link
Collaborator

That's a good catch 👍🏻 Please review: #651

@MaxymVlasov MaxymVlasov added bug_with_workaround Something isn't working but there is a workaround and removed bug Something isn't working area/local_installation labels Mar 25, 2024
@MaxymVlasov
Copy link
Collaborator

MaxymVlasov commented Mar 25, 2024

Workaround

Remove quotes:

- --env-vars=KEY=value
- --env-vars=KEY2=value with spaces
- -e KEY4=value with spaces

That's all that will work, but there a chance of adding nonindependent spaces like

- --env-vars=KEY3= value with spaces         

Which will be rendered to KEY3=' value with spaces', hopefully, spaces after always ignored

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug_with_workaround Something isn't working but there is a workaround
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants