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(exec): Whitelist LC_ALL and LANG #5239

Merged

Conversation

zharinov
Copy link
Collaborator

Ref #5062

@zharinov
Copy link
Collaborator Author

@rarkins
Copy link
Collaborator

rarkins commented Jan 28, 2020

Can you explain this more? We need to pass env vars to child process but not to Docker itself via -e

@zharinov
Copy link
Collaborator Author

I thought, pipenv pass them to Docker already and we should do for all. Well, here we have two options: (1) extend basic vars with LANG and LC_ALL, passing it to Docker explicitly when needed or (2) pass it always as done in this PR.

@rarkins
Copy link
Collaborator

rarkins commented Jan 28, 2020

Thanks, I understand better. I wonder if they’re only needed for child process when using global binary source. In which case, they should be safe to include on the default whitelist. If they’re needed inside the container then we should build them into the container if not already

@zharinov
Copy link
Collaborator Author

By the way, I doubt LC_ALL and LANG actually needed for pipenv. This is the only place we pass basic vars into container.

@rarkins
Copy link
Collaborator

rarkins commented Jan 28, 2020

By the way, I doubt LC_ALL and LANG actually needed for pipenv. This is the only place we pass basic vars into container.

I suspect there were only needed for binarySource=global. Let's add them to our whitelisted env vars for child process and not pass them into the container itself.

@rarkins rarkins changed the title feat(exec): Whitelist some safe basic variables for Docker fix(exec): Whitelist LC_ALL and LANG Jan 29, 2020
@rarkins rarkins merged commit aa49a21 into renovatebot:master Jan 29, 2020
@rarkins rarkins deleted the feat/whitelisted-docker-basic-vars branch January 29, 2020 05:29
@renovate-release
Copy link
Collaborator

🎉 This PR is included in version 19.112.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants