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

Support for loading config object from "<(/usr/bin/envsubst < template)" #187

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

LucidOne
Copy link

@LucidOne LucidOne commented Jul 6, 2019

configobj has an issue being passed a file descriptor (/dev/fd) via process substitution.

os.path.isfile("/dev/fd/{{ anything }}") returns False for file descriptors while os.path.exists() returns True.

test.py

#!/usr/bin/python
import sys
import configobj

print configobj.ConfigObj(sys.argv[1])

I'll update the PR with a test when I get a chance

$ cat foo.ini 
shell = /bin/bash
$ cat bar.ini.template 
shell = $SHELL
$ ./test.py foo.ini 
{'shell': '/bin/bash'}
$ ./test.py <(envsubst < bar.ini.template )
{'shell': '/bin/bash'}

This is needed in cloud environments where you may not want to store passwords on disk in an unencrypted form or have variables passed into a container environment.

@coveralls
Copy link

coveralls commented Jul 6, 2019

Coverage Status

Coverage decreased (-47.9%) to 33.926% when pulling ae4b6ab on LucidOne:process_substitution into 45fbf1b on DiffSK:master.

src/configobj/__init__.py Outdated Show resolved Hide resolved
@LucidOne LucidOne force-pushed the process_substitution branch from d6e42cb to 6d166d9 Compare July 6, 2019 07:47
LucidOne added a commit to LucidOne/configobj that referenced this pull request Jul 6, 2019
@LucidOne LucidOne force-pushed the process_substitution branch from 6d166d9 to 9d7a288 Compare July 6, 2019 07:51
@LucidOne LucidOne force-pushed the process_substitution branch from 19db23a to ae4b6ab Compare July 6, 2019 08:29
@LucidOne
Copy link
Author

LucidOne commented Jul 6, 2019

I'm not sure why coverage dropped, but it looks like the tests work.
Let me know if you have any other feedback.

@LucidOne
Copy link
Author

Is there anything left to get this merged?

@LucidOne
Copy link
Author

LucidOne commented Nov 9, 2019

Any update on this?

@LucidOne
Copy link
Author

Is it possible to get this merged? I'd like to improve the letsencrypt certbot docker container for doing dns based wildcard cert requests and this is blocking me.

@LucidOne
Copy link
Author

Would it help to rebase this?

@jelmer
Copy link
Collaborator

jelmer commented Nov 28, 2024

Hi, what are the security implications of these changes? How trusted are files read by configobj today?

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.

4 participants