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

feature request: Allow config parser to detect environment variables #397

Open
andrewzah opened this issue Mar 13, 2018 · 4 comments · May be fixed by #1037
Open

feature request: Allow config parser to detect environment variables #397

andrewzah opened this issue Mar 13, 2018 · 4 comments · May be fixed by #1037
Labels
feature needs-contributor Someone needs to implement this. Help wanted! needs-decision Architectural/Behavioral decision by maintainers needed server (Python) server code
Milestone

Comments

@andrewzah
Copy link

andrewzah commented Mar 13, 2018

@ix5 ix5 added the server (Python) server code label Dec 27, 2021
@ix5
Copy link
Member

ix5 commented Dec 27, 2021

I'd be wary of having Isso be configured from multiple places. For now, it already reads ISSO_SETTINGS to find its own config files.

Having too many possible scenarios of mixed configs and inputs creates an untenable situation for testing. A bug in the parsing of another env variable slipped completely under the radar for that reason, see #720

You'd also have to deal with which config source would take precedence and what to do with duplicates. Parsing env variables, which is untrusted input that potentially any user can set, also creates a bit of a security nightmare.

I'm doing to close this for now, unless you have a valid reason for this feature to be implemented.

@ix5 ix5 closed this as completed Dec 27, 2021
@andrewzah
Copy link
Author

andrewzah commented Dec 27, 2021

  1. Env vars allow people to check config files into version control. I would like to be able to do this, and specify secrets as needed via env vars.
  2. Env vars are not less secure than reading files. Neither method provides security if the system is compromised. Only the user and root can see them, unlike files and directories which also need to be managed via permissions.
  3. Env vars take priority over files typically. Just pick one and state it in the documentation. Allowing interpolation in the config file itself solves this issue.
  4. "Parsing env variables, which is untrusted input that potentially any user can set" the vast majority of the computing world is completely unsecure, then. Just about every service that I use allows config setting via env vars.

Not allowing env vars doesn't provide any additional security if the system is compromised, and it makes it harder for system administration because I can't check the config into source control systems so I have to manage it by hand every time I (re)install isso (via docker, etc).

@ix5 ix5 reopened this Dec 29, 2021
@ix5 ix5 added needs-contributor Someone needs to implement this. Help wanted! needs-decision Architectural/Behavioral decision by maintainers needed labels Dec 31, 2021
@ix5
Copy link
Member

ix5 commented Feb 13, 2022

Having thought about this a bit, your arguments convince me.
Would you like to submit a PR that introduces such ways of configuring Isso?

@ix5 ix5 added this to the 0.13 milestone Feb 13, 2022
@ix5
Copy link
Member

ix5 commented May 1, 2022

Related: #763

@ix5 ix5 modified the milestones: 0.13, 0.14 May 24, 2022
@ix5 ix5 modified the milestones: 0.14, 0.13.1 May 9, 2024
pkvach added a commit to pkvach/isso that referenced this issue Sep 27, 2024
- Updated `IssoParser` to support environment variable substitution.
- Enhanced documentation to include details on using environment variables in the configuration file.
- Added unit tests to verify the functionality of environment variable substitution in `IssoParser`.

Closes isso-comments#397
@pkvach pkvach linked a pull request Sep 27, 2024 that will close this issue
5 tasks
pkvach added a commit to pkvach/isso that referenced this issue Sep 27, 2024
- Updated `IssoParser` to support environment variable substitution.
- Enhanced documentation to include details on using environment variables in the configuration file.
- Added unit tests to verify the functionality of environment variable substitution in `IssoParser`.

Closes isso-comments#397
pkvach added a commit to pkvach/isso that referenced this issue Sep 27, 2024
- Updated `IssoParser` to support environment variable substitution.
- Enhanced documentation to include details on using environment variables in the configuration file.
- Added unit tests to verify the functionality of environment variable substitution in `IssoParser`.

Closes isso-comments#397
pkvach added a commit to pkvach/isso that referenced this issue Oct 2, 2024
- Updated `IssoParser` to support environment variable substitution.
- Enhanced documentation to include details on using environment variables in the configuration file.
- Added unit tests to verify the functionality of environment variable substitution in `IssoParser`.

Closes isso-comments#397
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature needs-contributor Someone needs to implement this. Help wanted! needs-decision Architectural/Behavioral decision by maintainers needed server (Python) server code
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants