feat: get api key from OPENAI_API_KEY environment variable

[loiccoyle]

Try to get the api key from the OPENAI_API_KEY environment variable before prompting user.

This also avoids storing active api keys in unencrypted text files.

[kid-gorgeous]

This is your personal API Key. For security reason, it should stay private.

[loiccoyle]

Yeah that's the point, if you read from environment variables, you don't have to write it to file an unencrypted file, which is the current behaviour.

[TheR1D]

Hi @loiccoyle, thank you for the PR. While the suggestion make sense, it will not allign with comming changes, since we are planing to add option --reset-key #30 which will rewrite a key in the file. In this case it would be difficult to set new envOPENAI_API_KEY from Python process, because environment variables exist in a per-process memory space.

[loiccoyle]

In this case it would be difficult to set new envOPENAI_API_KEY from Python process, because environment variables exist in a per-process memory space.

But you don't need to set a new environment variable. The way I see it is if the user has the OPENAI_API_KEY variable set then it indicates that the user is managing their key and shell_gpt should simply look it up.

This PR doesn't break the current implementation of prompting and writing it to file if the variable is not set. So I think the upcoming --reset-key option can simply work as expected, and do nothing if the env var is set (maybe show a relevant warning/error msg). What do you think ?

I opened this PR because I use a few chatgpt integrations (https://github.com/not-poma/lazyshell, https://github.com/jackMort/ChatGPT.nviml) and they all read the OPENAI_API_KEY and I was surprised when shell_gpt didn't.

Edit: bad links

[TheR1D]

I meant --reset-key doesn't make sense when you can't change a key in env, but I agree that it can workout with some warning message. Please add changes in README.md, metion the optional env api key setup in Installation section.

[TheR1D]

@eric-glb These changes might be interesting for you 🙂 We would be able to pass OPENAI_API_KEY as env to the container.

[kid-gorgeous]

As a client, why do I need such a vulnerability in this software? I still don't believe this change is necessary implementation.

--reset-key is a better alternative in situations where you would like to control API directly.

[TheR1D]

Please add changes in README.md, metion the optional env api key setup in Installation section.

[loiccoyle]

As a client, why do I need such a vulnerability in this software? I still don't believe this change is necessary implementation.

--reset-key is a better alternative in situations where you would like to control API directly.

What vulnerability? Writing an active API key to an unencrypted file?

[loiccoyle]

Please add changes in README.md, metion the optional env api key setup in Installation section.

Sounds good, I'll do it shortly.

[kid-gorgeous]

This just seems like a vulnerability, is there any way to regulate something more secure before adding this PR?

[TheR1D]

This is a common way to store keys on the system. And it doesn't cancel implementation of --reset-key. Also it is optional, if you have doubts, just store key in the file.

[eric-glb]

@eric-glb These changes might be interesting for you 🙂 We would be able to pass OPENAI_API_KEY as env to the container.

I'll manage the docker part once the OPENAI_API_KEY use is merged.
From my understanding it's just a small amendment to add in the README.md.

(Edit: grammar and more details).

[loiccoyle]

FYI: I already added the instructions to the readme.

[TheR1D]

From my understanding it's just a small amendment to add in the README.md.

I would remove api key volume, and use env variable instead. Plus README.md updates.

[eric-glb]

#63, feel free to modify as you wish ^^