Repository navigation
Feature: file-based secrets from environment - #3568
parasite-lost wants to merge 1 commit into
Conversation
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Security | 1 critical |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| if file == "" { | ||
| return "", true, fmt.Errorf("no file provided: %s=", envVariable) | ||
| } | ||
| content, err := os.ReadFile(file) |
There was a problem hiding this comment.
Codacy Static Code Analysis complains that this line is not up to standards as it would allow reading user-defined files.
But this is exactly the intent - let the user (admin in this case) provide paths to arbitrary files that should be read to provide sensitive values.
question: how can I suppress this finding?
|
To my eyes the comma syntax is a bit weird, it looks like a list (especially when there's only one env var) Two thoughts on this:
|
That's the syntax of envdecode struct tags. You can already define default values by appending the definition with
Using config files feels a bit unwieldy and verbose as (if I understand things correctly) you have to provide the full configuration including default settings. Plus the documentation states that the aim is that opencloud can easily be configured by setting just the necessary parameters via environment variables.
I'm fine with adding the |
aduffeck
left a comment
aduffeck
left a comment
There was a problem hiding this comment.
While I only really see a use case for file-based keys and secrets I would also prefer to support the _FILE vars in general for simplicity. That also avoids issues with forgotten file tags in the future and os.LookupEnv is cheap enough to not bother I think.
As a micro optimization we could maybe spare a few cycles by checking the "regular" env vars first and only fall back to the _FILE variant if it isn't set, though.
Support providing configuration parameters file-based via the environment in envdecode. For every environment variable supported by any opencloud component support additionally the same environment variable suffixed with `_FILE` to retrieve the configuration parameter from file instead of directly from the environment. This allows configuring opencloud components more securely, preventing sensitive values being leaked via the environment and providing sensitive values for example via systemd credentials or file-based container secrets that can be encrypted at rest. Example: instead of setting `MY_SENSITIVE_VALUE=secret-password` in the environment you can now set `MY_SENSITIVE_VALUE_FILE=/run/secrets/sensitive-value` where `/run/secrets/sensitive-value` (arbitrary path) is a file containing `secret-password` (trailing newlines are ignored).
5ee79f4 to
04245f1
Compare
Works for me - this makes everything much simpler. Indeed, there are several environment variables that are repeatedly parsed by different components - missing one There are however a couple of environment variables that already reference files, such as I've adjusted my changes to code and (envdecode) documentation accordingly. |
3 participants
Description
Add support for all environment variables providing sensitive values to provide a path to a file via
_FILEsuffixed environment variable instead where the file contains the sensitive value.This can be used for example in conjunction with container secrets or systemd credentials to securely provide sensitive values.
Related Issue
This is also related to #3034 (PR #3034).
Motivation and Context
Securely provide sensitive values by files (referenced via environment) instead of leaking sensitive values in the environment.
How Has This Been Tested?
Types of changes
While maintaining full backwards compatibility (all prior options for configuring parameters haven't changed):
_FILEsuffixed environment variables via envdecode struct tags (by adding,fileparameter to the environment variable definition)Checklist: