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

Fix for #5134 #5139

Merged
merged 4 commits into from
Jun 21, 2022
Merged

Fix for #5134 #5139

merged 4 commits into from
Jun 21, 2022

Conversation

ghost
Copy link

@ghost ghost commented Jun 20, 2022

Note: This is my first time ever contributing to an open source project. I'm super grateful for the work all of you are putting into the project. Keep it up!

The issue

According to issue #5134, environment variables are expanded when generating a requirements.txt file with pipenv requirements. This leads to credentials being leaked.

The fix

The culprit can be found here.
How I understand the code, when calling pipenv requirements the lockfile is loaded using the lockfile_content property of the project object. This contains the lockfile as it used during runtime.
However, when generating a requirements.txt file this is not what we want, since environment variables are expanded.
Luckily, the lockfile can be loaded without expanding environment variables.
def load_lockfile(self, expand_env_vars=True):
https://github.com/pypa/pipenv/blob/main/pipenv/project.py#L974

This is what I have done.

The checklist

  • Associated issue
  • A news fragment in the news/ directory to describe this fix with the extension .bugfix, .feature, .behavior, .doc. .vendor. or .trivial (this will appear in the release changelog). Use semantic line breaks and name the file after the issue number or the PR #.

@oz123 oz123 self-requested a review June 21, 2022 12:27
@oz123 oz123 merged commit e276239 into pypa:main Jun 21, 2022
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.

2 participants