Bug 256081
| Summary: | security/vaultwarden: Level up ports compliance | ||||||
|---|---|---|---|---|---|---|---|
| Product: | Ports & Packages | Reporter: | Bernard Spil <brnrd> | ||||
| Component: | Individual Port(s) | Assignee: | Michael Reifenberger <mr> | ||||
| Status: | Closed FIXED | ||||||
| Severity: | Affects Some People | CC: | wcarson.bugzilla | ||||
| Priority: | --- | Flags: | bugzilla:
maintainer-feedback?
(mr) koobs: merge-quarterly? |
||||
| Version: | Latest | ||||||
| Hardware: | Any | ||||||
| OS: | Any | ||||||
| Attachments: |
|
||||||
Comment on attachment 225172 [details]
git diff for security/vaultwarden
Approved by: portmgr (blanket: ports compliance)
MFH: <branch> (blanket: ports compliance)
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. (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. (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 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 (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... 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.) 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(-) |
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