Allow using env vars to set config values

[SydneyUni-Jim]

What

Create a syntax for config files — such as, ojs.config.inc.php — that allows setting a configuration value using an environment variable.

I propose the syntax be {env:VAR_NAME}¹. This would substitute the value of the environment variable VAR_NAME, or the blank string if it is not set. The value from the environment variable undergoes trimming and type conversion, the same as configuration values not taken from an environment variable. The value of the environment variable does not undergo further substitution.

The syntax is quiescent inside '…' and "…" literal strings.

The {…} in the syntax clearly delimits it, and the env: clearly marks it as a reference to an environment variable.

The syntax is unlikely to have appeared organically in current configuration files. If it were it can continue to be used literally by turning the configuration value into a '…' or "…" literal string.

I propose this lands in version 3.5.0 as it is still a potentially breaking change.

Why

There are configuration values that cannot, or even must not, be built into a container image.

The pkpofficial/ojs docker image follows the standard practice of using environment variables in these cases. It achieves this via the ojs-variable command, which uses sed to substitute an environment variable into the configuration file.

While this works, it hinders running containers with a read-only root filesystem, a security best practice.

How

I propose a change to readConfig in PKP\config\ConfigParser, which would add code before line 72 to match the syntax in $value and substitute the value of an environment variable. Line 72 would then use the result of that process.

pkp-lib/classes/config/ConfigParser.php

Lines 68 to 78 in 65607cc

	if (preg_match('/^[\"\'](.*)[\"\']$/', $value, $matches)) {
	// Treat value as a string
	$value = stripslashes($matches[1]);
	} else {
	preg_match('/^([\S]*)/', $value, $matches);
	$value = $matches[1];
	// Try to determine the type of the value
	if ($value === '') {
	$value = null;
	} elseif (is_numeric($value)) {

Here is an example of the proposed syntax in situ.

[general]
base_url = {env:OJS_BASE_URL}

If the OJS_BASE_URL environment variable was https://example.com/ojs, Config::getVar("general", "base_url") would return the string https://example.com/ojs.

Stretch goals

Embedding

Allow the syntax to be embedded within other text. Allow multiple occurrences of the {env:…} syntax when constructing a value.

[general]
base_url = https://{env:SERVERNAME}/{env:BASEPATH}

If the SERVERNAME environment variable was example.com and BASEPATH was ojs, Config::getVar("general", "base_url") would return the string https://example.com/ojs. If BASEPATH was instead of unset or blank, the returned string would be https://example.com/.

Default values

Extend the syntax to allow {env:VAR_NAME:-VALUE}. If VAR_NAME is unset or empty, VALUE is used instead. I took this syntax from POSIX shell parameter expansion. However, I propose VALUE be used literally. This precludes complex syntaxes like {env:VAR1:-{env:VAR2:-VALUE}}.

This is an example of the extended syntax in situ.

[debug]
show_stacktrace = {env:OJS_DEBUG:-Off}
display_errors = {env:OJS_DEBUG:-Off}

If OJS_DEBUG is unset or an empty value — via OJS_DEBUG= in a .env file, for example — show_stacktrace and display_errors would both have the boolean value false. If OJS_DEBUG is true or on, show_stacktrace and display_errors would both have the boolean value true. If OJS_DEBUG is X both show_stacktrace and display_errors would have the string X, as is current behaviour.

Delete after use

Extend the syntax to allow {env:VAR_NAME:delete}. Any environment variable referenced with this syntax is deleted from the environment after reading the whole of the configuration file.

Some environment variables may contain secrets. An example of this might be the database password. Having secrets in environment variables risks the secrets being exposed should the environment become exposed. Being able to delete these environment variables help to mitigate this risk.

Future expansion

These are not part of this proposal, but are mentioned here because they were considered.

• Create a {file:PATH} syntax that would set a configuration variable to the contents of the file at PATH. This would facilitate using docker compose secrets in configuration files.
• Create a {config:SECTION:KEY} syntax that would set a configuration variable based on prior configuration variable.

Footnotes

1. I had considered the somewhat more common ${env:VAR_NAME} syntax. But leaned towards something based on PHP's variable substitution syntax instead. ↩

[SydneyUni-Jim]

I would be prepared to contribute the code for this proposal, including the stretch goals.

[SydneyUni-Jim]

This would be my proposed approach to implementing this.

I would create an interface for a configuration value substitution provider, and one implementation which would substitute values from environment variables.

I would create a generic processor for the syntaxes {prov:key}, {prov:key:-value}, and {prov:key:delete}. It would delegate getting and deleting to the configuration value substitution provider prov. But it would implement substituting value when the providers returns null or an empty string.

If there is no provider for prov, the whole syntax remains as is. This enhances backwards compatibility with prior organic uses of {…}, but at the expense of error detection.

This would create the bones for future file and config configuration value substitution providers.

[asmecher]

@marcbria, tagging you on this as our main docker/containerization agitator; I think you had made similar proposals. Could you cross-link those, and have a think about this proposal?

[marcbria]

Thanks Jim. So happy to hear about undusting this. :-)

I think best most relevant post about ENV vars in OJS is #7068:
Consider implementing the phpdotenv usage to handle configuration"

In the post Henrique suggest to "use the bullet-proof package vlucas/phpdotenv to handle the .env parsing and setting."

Then the thread evolves to covers very interesting related topics (move from ini to yaml/toml, save config in DB, combine multiple config files, security concerns...) but made us take distance from the original proposal and it was never implemented.

This final post from Nate is a good summary of the hierarchy we like to apply, that I'm sure will make Jim as happy as me:
#7068 (comment)

So, if I understood well, we have two proposals on the table:

• Jim's: That suggest to parser the config.inc.php to capture "env:" prefix and load the proper ENV var.
• Henrique's: That suggest using phpdotenv library and let ENV vars overwrite the config ones.

I like Jim's idea (that offers one benefit that is removing used vars) but IMO, will be better using an existing library that let us release all the potential of the ENV files (aligned with the 12 factor) in a more standardized way.

Related conversations:
I'm not a DEV so I'm unsure if #10027 overlaps with the work we are trying to accomplish here.
At same time, we need to keep in mind that whatever we do config file need to be valid or OJS will break (#9781)

Probably @jonasraoni and @AndrewGearhart could be interested in this Discussion too?

Update:
Just noticed the issue was converted to a Discussion here and @withanage extended it with a summary of the work they did in Helskinki in 2022.

The group made a great summary of the topics related and suggest config improvements but also suggest going with phpdotenv.

[AndrewGearhart]

Hey, @marcbria ... thanks for pinging me.

I appreciate many aspects of Jim's idea, but I have some reservations. While the {env:VAR_NAME} syntax is creative, I’m not keen on modifying the config parser in this way to adapt to a relatively non-standard syntax.

As a side note, I fundamentally disagree that environment variables containing secrets are less secure than other secret storage mechanisms in a containerized environment, like writing values to disk or using an application's global variables. Deleting environment variables after reading them does offer some security benefits, but it's secondary since an attacker who compromises the application can access both the variables and settings. Proving one method or another would require implementation details to be argued, meaning these are functionally equivalent at face value. In many cases, I appreciate structured data config files (e.g., YAML over JSON because of comments), but setting OJS settings from the environment provides multiple avenues for configuration management, aligning with best practices for deployments.

Without environment variables being available directly, a simple approach could be using phpdotenv or envsubst to parse environment variables in a config template. In some deployments, we've also overridden config.inc.php with mounts in Kubernetes deployments for flexibility. These would support Jim’s point about a read-only file system in containerized applications being a security net gain. However, I feel the best approach is likely a configurable config file that dynamically pulls settings from the environment, allowing more deployment flexibility.

In other words, I think that we gain all the versatility of Jim's proposal through a simpler approach: protect the config file and make it PHP operable. This leverages PHP's native environment variable handling, simplifies the codebase, and aligns with PHP practices that are standard in many other frameworks and applications.

[AndrewGearhart]

To clarify how I would protect the config.inc.php and allow for the config file to be operable: ...

Allow for a directive variable (let's say, "DYNAMIC_CONFIG_FILE" at the beginning of the config.inc.php that states an alternative .php file that could be used. The php file would be a true php file and would have variables set there. What the variables are set to and how would be up to the user that implements the OJS site. Use an environment variable, directly configure by using a string... however you do it... your call. Just use PHP syntax to set the information. This also allows for the ability to include other php files for differentiated environments as well. Lots of benefits of going this direction.

I'll go ahead and toss this part over into the discussion... as it has been a long time since I've contributed directly there. ;)

[SydneyUni-Jim]

I really like this proposal to allow config to be a .php file.

[SydneyUni-Jim]

Closing this in favour of #8015.