Bug 256081 - security/vaultwarden: Level up ports compliance
Summary: security/vaultwarden: Level up ports compliance
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Michael Reifenberger
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-05-22 13:29 UTC by Bernard Spil
Modified: 2021-06-30 16:26 UTC (History)
1 user (show)

See Also:
bugzilla: maintainer-feedback? (mr)
koobs: merge-quarterly?


Attachments
git diff for security/vaultwarden (4.72 KB, patch)
2021-05-22 13:29 UTC, Bernard Spil
koobs: maintainer-approval+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bernard Spil freebsd_committer 2021-05-22 13:29:54 UTC
Created attachment 225172 [details]
git diff for security/vaultwarden

Fix some issues with the port

 * Replace the bundled OpenSSL with the OS' SSL as per the crates warning message.
 * Replace user/group with WWWOWN and WWWGROUP proper
 * Use @sample for the sample file, reflect in pkg-message
 * Remove unneeded entries from pkg-plist
Comment 1 Kubilay Kocak freebsd_committer freebsd_triage 2021-05-23 02:33:17 UTC
Comment on attachment 225172 [details]
git diff for security/vaultwarden

Approved by: portmgr (blanket: ports compliance)
MFH: <branch> (blanket: ports compliance)
Comment 2 wcarson.bugzilla 2021-05-24 18:36:59 UTC
Hello, thank you for your work on this port Bernard, I'm very excited to start using it. I installed this yesterday with the proposed patch merged, and it creates some files that did not make sense to me. For example, it creates:

/usr/local/etc/vaultwarden/
/usr/local/etc/vaultwarden/rc.conf.d/
/usr/local/etc/rc.conf.d/
/usr/local/etc/rc.conf.d/vaultwarden.sample

I finally learned that this should be named "/usr/local/www/data/vaultwarden/.env", otherwise it is completely ignored. Furthermore, the latest .env file from upstream has a *lot* of additional tunables:

https://raw.githubusercontent.com/dani-garcia/vaultwarden/main/.env.template

I did this to clean this up for myself, but I'm unsure how to do it correctly to provide a ports patch:

# rm -rf /usr/local/etc/vaultwarden /usr/local/etc/rc.conf.d
# ln -s /usr/local/etc/vaultwarden.conf /usr/local/www/data/vaultwarden/.env
# curl -s https://raw.githubusercontent.com/dani-garcia/vaultwarden/main/.env.template > /usr/local/etc/vaultwarden.conf 

Oh, also in /usr/local/etc/rc.d/vaultwarden there's a typo that prevents startup:

    : ${vaultwarden_chdir=%%PREFIX%%/www/vaultwarden}
    : ${vaultwarden_user:=%%WWWOWN%%}
    : ${vaultwarden_user:=%%WWWOWN%%}

${vaultwarden_user} is entered twice, the second line should be:

    : ${vaultwarden_group:=%%WWWGRP%%%}

Should there be a ":=" on vaultwarden_chdir? I don't know the effect, but it seems to work without it.
Comment 3 Bernard Spil freebsd_committer 2021-05-24 19:20:25 UTC
(In reply to wcarson.bugzilla from comment #2)



> /usr/local/etc/vaultwarden/
> /usr/local/etc/vaultwarden/rc.conf.d/

Haven't figured this out. Not used as far as I can see, but unclear where they originate.

> /usr/local/etc/rc.conf.d/
> /usr/local/etc/rc.conf.d/vaultwarden.sample

This is the desired behavior, see https://docs.freebsd.org/en/books/porters-handbook/book.html#plist-keywords-sample. It will _not_ overwrite the /usr/local/etc/rc.conf.d/vaultwarden on install, but it will copy the sample when the /usr/local/etc/rc.conf.d/vaultwarden file does not exist yet.
I haven't seen this rc.conf.d earlier either! I guess FreeBSD added loading environment variables for rc-scripts.

> I finally learned that this should be named "/usr/local/www/data/vaultwarden/.env"

the behavior I've seen is that /usr/local/etc/rc.conf.d/vaultwarden is used (by the rc file?) The sample should reflect the template you mentioned.

> Oh, also in /usr/local/etc/rc.d/vaultwarden there's a typo that prevents startup

Some git snafu?! This is in the patch

--- a/security/vaultwarden/files/vaultwarden.in
+++ b/security/vaultwarden/files/vaultwarden.in
@@ -16,8 +16,8 @@ rcvar=vaultwarden_enable
 load_rc_config ${name}
 
 : ${vaultwarden_chdir=%%PREFIX%%/www/vaultwarden}
-: ${vaultwarden_user:=www}
-: ${vaultwarden_group:=www}
+: ${vaultwarden_user:=%%WWWOWN%%}
+: ${vaultwarden_group:=%%WWWGRP%%}
 
 # This is the tool init launches
 command="/usr/sbin/daemon"

And yes, vaultwarden_chdir should have a ':='. These are ultimately shell scripts. Check https://www.freebsd.org/cgi/man.cgi?sh (search for :=) to see why.
Comment 4 Bernard Spil freebsd_committer 2021-05-24 19:29:23 UTC
(In reply to Bernard Spil from comment #3)

TIL... rc.conf.d

See https://www.freebsd.org/cgi/man.cgi?query=rc.conf

What happens is that the file is included by rc when loading the service. Thus, it makes these environmen-variables available to the service binary that's started.

Smart to use this for things that use Docker-like startup using environment vars
Comment 5 wcarson.bugzilla 2021-05-24 19:36:53 UTC
Oh, I see. TIL as well. In my various testing I must have missed that particular combination. (I think the two different rc.conf.d directories threw me.) For it to work, you must have the 'export VARIABLE' line as well (which is there if you use the .env.template included in the port, but not if you've updated it as I had).

I'm not sure it's particularly sustainable to use the rc.conf.d method, as you'll have to fetch and massage the .env.template file regularly and add the export commands for it to work. Additionally, you'll still see a notice that a .env file was not found on startup:

May 24 12:29:41 system vaultwarden[30046]: [INFO] No .env file found.

Also, I think I typo'd the location vaultwarden looks for .env. The correct location is:

/usr/local/www/vaultwarden/.env
Comment 6 Bernard Spil freebsd_committer 2021-05-24 20:08:15 UTC
(In reply to wcarson.bugzilla from comment #5)

We could create an empty .env file in stead of the rc.conf.d/vaultwarden file.

It is not uncommon for web-apps to store their configuration in WWWDIR, but generally this is frowned upon from a FreeBSD perspective (config goes into ${PREFIX}/etc).

I've noticed that vaultwarden overrides the data in .env after the /admin interface is used... Count your blessings.
Guess this is a typical Docker thing? Can only pass environment, storage is ephemeral...
Comment 7 wcarson.bugzilla 2021-05-24 20:10:33 UTC
Yeah, I'm not sure what the right answer is. Perhaps what we have but with a link to the .env.template in a comment if the user wants to see all the available options? (It was not obvious to me how to locate the upstream source of the rc.conf.d/vaultwarden file.)
Comment 8 commit-hook freebsd_committer 2021-06-02 15:14:09 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=f097145a000b6ce646c4193be59368413d678518

commit f097145a000b6ce646c4193be59368413d678518
Author:     Michael Reifenberger <mr@FreeBSD.org>
AuthorDate: 2021-06-02 15:07:57 +0000
Commit:     Michael Reifenberger <mr@FreeBSD.org>
CommitDate: 2021-06-02 15:12:12 +0000

    security/vaultwarden: Level up ports compliance

    add some of the changes (except the openssl debundling) of the PR.

    PR:             256081

 security/vaultwarden/Makefile             | 8 ++++----
 security/vaultwarden/files/vaultwarden.in | 4 ++--
 security/vaultwarden/pkg-plist            | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)