Add support for file-based environment variables in environment-to-ini (v1.16 backport)

[aminosbh]

Improve environment-to-ini to allow for file content to be set as the value of an environment variable.
Useful when using docker secret and were the secret is mounted as a file in /run/secrets/<SECRET_NAME>.

Any settings in app.ini can be set or overridden with the content of a file by defining an environment variable of the form: GITEA__section_name__KEY_NAME__FILE that points to a file.

Backports #19857
Fixes #19856

[zeripath]

Hmm... isn't this just going to result in secrets being copied directly into the app.ini file - breaking the point of secrets...

[aminosbh]

@zeripath It's true that the content of the files will be present as plain text inside the app.ini file but that is also the case when mounting the docker secret as file inside the container. The intent of using this technique is to prevent storing secrets as plain text in the host (i.e. docker-file.yml) and also to prevent publishing the secret directly in an env var because they are in many cases logged or stored by monitoring/reporting tools.

[aminosbh]

@zeripath Furthermore when storing secrets in let's say docker-compose.yml most of the time it get pushed to a git repo for versioning purposes.

[zeripath]

I'm not suggesting that the situation prior to this PR is correct. Just that I am concerned that the solution proposed by this PR may not be the best solution - I'm not certain that people would intend that the secrets would be copied into the containers directly.

---

I have previously suggested two IMO very simple solutions :

1. We add another KEY to the root of the ini e.g. Cfg.Section("").Key("PARENT_FILE").String() which would be read and this Cfg file overlaid on top.
2. we allow multiple -c config files the order of which is overlaid on top each one.

These options would require that the secrets file be an ini - but I'm not certain that there's a great argument against this.

Here are three more complex options:

• Formally migrate the settings could be filed this way to a [secrets] section which could then have simple __FILE endings.
• Make every key have an optional __FILE ending which would automatically read from a file
• Do the work and checking to ensure that the environment is passed down correctly in git hooks and in ssh etc. then we could just check the environment correctly.

[zeripath]

In any case I don't think this should be backported to 1.16

[aminosbh]

@zeripath Thank you for your feedback

• For the first point I think that having multiple config files (one overlaid on top of the other) will be a great feature in all cases even with non-secret settings (e.g. dev/prod settings). In case we would like to benefit from docker secret feature It will be doable to put all secrets in one ini file (or multiple) and save the content using docker secret and have it mounted as file and loaded with the mechanism you described. But there is one drawback in using ini file format for secrets which is that secrets (managed with docker secret or other tools like Kubernetes or HashiCorp Vault) will not be re-usable with other containers, let's say PostgreSQL witch expects a dedicated file for the password in plain text (env var POSTGRES_PASSWORD_FILE).

• Concerning your suggestion of "Make every key have an optional __FILE ending which would automatically read from a file" I like the idea and I think it's the best suitable for the long run. I think also that having an abstraction over the global settings can facilitate their propagation to the other components. I mean that they do not have to know that the config in question comes from the content of a file or directly offered in the ini file.

• Migrating the secrets to a dedicated section could be tempted but this will have bigger implications specially when it comes to the migration from the previous config format to the new one. So I think that it's not suitable for a 1.x version (or maybe it is, depending on your versioning policies).

I will summarize the usability and security implications of the discussed solutions:
🟢 for positive point, 🔴 for negative point and empty when N/A or could be prevented

1. Save secrets in ini file (Current implementation)
2. Save secrets in overlaid dedicated ini file
3. Make every key have an optional __FILE ending which would automatically read from a file.
   a. Implemented inside gitea: Load secrets from files to memory.
   b. Implemented inside environment-to-ini: Load secrets from file and populate ini file inside the container. (This PR)
4. Make a dedicated secrets section in init file and load from files to memory.

Implication	1 (Current)	2	3.a	3.b (PR)	4
Secrets as plain text in the host	🔴
Secrets encrypted in the host (docker secret, Kubernetes, HashiCorp Vault, ...)		🟠	🟢	🟢	🟢
Secrets grouped with the other settings	🔴
Secrets separated from the other settings[*]		🟢	🟢	🟢	🟢
Secrets as plain text inside the container[**]	🔴	🔴	🔴	🔴	🔴
Secrets as plain text in volumes[***]	🟡			🟡
Secrets re-usable with other containers (wide usage)			🟢	🟢	🟢
Major update to ini file					🔴
Update only environment-to-ini				🟣

🟠 : Managing the whole overlaid ini file (dedicated for secrets) with docker secret
🟣 : I don't know if it could be a positive or negative point (in the short run) but the risk of breaking gitea is reduced and, for sure, for the long run it is better to implement it gitea-side.
[*]: When the secrets are separated from the general settings they could be managed by docker secret and mounted only when running the container.
[**]: In all cases the secrets mounted as files will be present as plain text inside the container.
[***]: Having secrets as plain text in volumes could be a security risk but could be mitigated if using an encrypted filesystem and setting the correct file-permission for the volume directory to prevent unprivileged processes from accessing it.

I initially created a PR for the main branch #19857 so please consider that one instead. Since I was using v1.16 and since this feature was not implemented gitea-side I was tempted by integrating this feature in the v1.16.

I'm going to edit the associated issue #19856 to describe this feature regardless of the implementation.

[lunny]

Closed as 1.16 is end support.