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

Add "-s" option to delete dotenv files on command failure #140

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vmarkovtsev
Copy link

Rationale: sometimes we want to secure our environment variables so that the next command cannot access them.
We can chain the two operations:

godotenv cmd
rm .env

The problem here is if the command fails, we do not remove the file.

We can code some shell:

godotenv cmd || rm cmd && exit 1

There are two problems now:

  1. We lose the original exit code.
  2. We have to write this for each executed command.

godotenv -s solves everything.

Rationale: sometimes we want to secure our environment variables so that the next command cannot access them.
We can chain the two operations:

```
godotenv cmd
rm .env
```

The problem here is if the command fails, we do not remove the file.

We can code some shell:

```
godotenv cmd || rm cmd && exit 1
```

There are two problems now:

1. We lose the original exit code.
2. We have to write this for each executed command.

`godotenv -s` solves everything.
@joho
Copy link
Owner

joho commented Apr 26, 2021

I'm sorry, call it a failure of imagination on my part, but what specifically is the use-case here?

The scope of this library is to provide developer convenience when implementing apps the "12 factor" way (for all the later uncovered sins of that approach) and I don't see how this fits within the intended purpose.

@vmarkovtsev
Copy link
Author

Sure. Let me note that this PR changes the CLI code only, as I predicted that changing the library for my specific use case would be too much.

As you may have heard, codecov had a security incident and their bash uploader was compromised. Many had the following GHA config:

env:
    MY_SECRET: ${{ secrets.MY_SECRET }}

my_job:
  - name: some shell
    run: |
      commandAWantsSecret ...
      commandBWantsSecret ...
      commandCWantsSecret ...
  - name: codecov
    uses: codecov/codecov-action@v1

and their MY_SECRET was recorded and hijacked by codecov. To overcome the threat in the future with another third-party GHA or used tool in that GHA, we started to write this:

my_job:
  - name: some shell
    run: |
      MY_SECRET=${{ secrets.MY_SECRET }} commandAWantsSecret ...
      MY_SECRET=${{ secrets.MY_SECRET }} commandBWantsSecret ...
      MY_SECRET=${{ secrets.MY_SECRET }} commandCWantsSecret ...

and this works. However, if there are tens of secrets, this quickly gets notorious. I did some googling and found some statically linked binaries to convert .env files to environment variables for the specified command and found godotenv. Thanks to my patch, it becomes possible to setup the environment only once:

my_job:
  - name: some shell
    run: |
      echo 'MY_SECRET=${{ secrets.MY_SECRET }}' >.env
      godotenv commandAWantsSecret ...
      godotenv commandBWantsSecret ...
      godotenv commandCWantsSecret ...
      rm .env

I hit two problems though:

  • If some command fails, the CI terminates and executes post-actions from the involved third-party actions, and they access my .env file that I don't remove.
  • godotenv does not forward the exact status code, so if some command fails, I cannot always tell if it was due to OOM, for example.

The first problem can be worked around by intimidating shell scripting, the second is not solvable at all. Hence the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants