Modify Entrypoint Script to Support Docker Secret for Admin Password

[jj15asmr]

The Problem

When supplying the admin password as a Docker (Compose) secret and using the YOURLS_PASS_FILE environment variable to specify the path at which it is mounted in the container, automatic password hashing fails, and the following message is displayed in the admin panel:

Could not auto-encrypt passwords. Error was: "preg_replace problem".

It took me longer than I'd like to admit to trace it, but it appears to stem from the following part of the container-entrypoint.sh script:

containers/images/yourls/container-entrypoint.sh

Lines 77 to 82 in da022d5

	: "${YOURLS_USER:=}"
	: "${YOURLS_PASS:=}"
	if [ -n "${YOURLS_USER}" ] && [ -n "${YOURLS_PASS}" ]; then
	result=$(sed "s/ getenv_container('YOURLS_USER') => getenv_container('YOURLS_PASS'),/ \'${YOURLS_USER}\' => \'${YOURLS_PASS//&/\\&}\',/g" user/config.php)
	echo "$result" > user/config.php
	fi

It's checking if the YOURLS_USER and YOURLS_PASS environment variables are both defined, and if so, overwriting the dynamic username/password array in the config file with their static values. YOURLS_PASS isn't defined when a secret is used for the password, and so this whole block isn't evaluated, causing the hashing process to fail as YOURLS can't actually find a match for the plaintext password in the config file.

The (Proposed) Fix

The relevant chunk of the entrypoint script has been modified to also check for the YOURLS_PASS_FILE environment variable. If filled in, the contents of the file are retrieved and set as YOURLS_PASS, allowing the config file to be edited as intended.

[LeoColomb]

Thanks for submitting this pull request, @jj15asmr.
When the user is set using the password file env, the encryption should not be triggered.
Instead of adding a workaround, I'd prefer fixing the upstream "bug".
See YOURLS/YOURLS#4066

[jj15asmr]

Thanks for submitting this pull request, @jj15asmr. When the user is set using the password file env, the encryption should not be triggered. Instead of adding a workaround, I'd prefer fixing the upstream "bug". See YOURLS/YOURLS#4066

Thank you for the quick reply!

Your point of fixing the upstream "bug" actually brings me back to something that confused me a bit when I was first investigating this problem, and is probably worth pointing out now:

If my understanding is correct, the core yourls_maybe_hash_passwords() function should return false when yourls_is_user_from_env() returns true. This would appear to imply that no hashing should occur, even when the password was set using YOURLS_PASS (and not just YOURLS_PASS_FILE).

The entrypoint script would appear to overlook this intention, though, by statically writing the contents of YOURLS_USER and YOURLS_PASS to the config file, which would trigger automatic hashing. This is what led me to extend the same behavior when YOURLS_PASS_FILE was used.

Basically, to me it seems that:

1. YOURLS core intends that no hashing should occur when the password is set via the environment.
2. The entrypoint script ignores this intention by statically writing those credentials to the config file.
3. Hashing occurs anyway because the password is now stored in plaintext.

Assuming the core's intended behavior is the "correct" one to follow, shouldn't the entrypoint not write the username/password directly to the config file and instead allow it be determined dynamically at runtime using getenv_container() (just like YOURLS_DB_PASS and YOURLS_DB_PASS_FILE are)?

I hope that all makes sense 😅. If I am totally misunderstanding something here, then I apologize.

[LeoColomb]

even when the password was set using YOURLS_PASS

No. I 100% agree this is extremely confusing, but note the difference between YOURLS_PASS and YOURLS_PASSWORD. The check in done on the latest, and the image is using the former.
I also agree there is probably rooms for improvment here, but I'd advocate to reduce complexity in the container init script than the opposite.

[jj15asmr]

No. I 100% agree this is extremely confusing, but note the difference between YOURLS_PASS and YOURLS_PASSWORD. The check in done on the latest, and the image is using the former.

I see... so the core yourls_is_user_from_env() function is essentially for purposes of running tests (as mentioned by you in the upstream PR discussion), and not really related to running YOURLS in a container?

I also agree there is probably rooms for improvment here, but I'd advocate to reduce complexity in the container init script than the opposite.

I am also on board with reducing complexity here. Do you think the following would be a clean way forward here?

• Remove lines 77 to 82 in the entrypoint script, which statically writes the username and password.
• Set the YOURLS_NO_HASH_PASSWORD constant to true in the container-specific config.php file.
• Close/abort the upstream PR modifying yourls_is_user_from_env().

This is what I did on my YOURLS container, and it seemed to work okay. But you're a lot more experienced around here than I am.