Restrict config file permissions to root and pimonitor group#35
Merged
Conversation
install.sh previously wrote /etc/pimonitor/config.yaml with mode 644, so any local user or compromised service could read the api_key secret, defeating the point of enabling API authentication. Install the config as 640 root:pimonitor and the directory as 750 root:pimonitor, and tighten the permissions of a pre-existing config on upgrade without touching its content. The service (running as user pimonitor) can still read the file via group membership. Closes #20 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ErDJ9Yxv9pHYHjF2a26Go6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
packaging/install.shinstalled/etc/pimonitor/config.yamlwith mode 644, so any local user or compromised service on the Pi could read theapi_keysecret, defeating the point of enabling API authentication.This PR tightens the installer:
install -m 640 -o root -g pimonitor(thepimonitorgroup already exists at that point — theuseradd --systemblock runs earlier in the script)./etc/pimonitoris set to750 root:pimonitoraftermkdir -p.640 root:pimonitorwithout touching the file's content.README.mdand in theapi_keycomment block ofpackaging/pimonitor.example.yaml(including thechown/chmodcommands for manually copied configs).The service (running as user
pimonitorper the systemd unit) can still read the config via group membership.Verified by exercising the exact fresh-install and upgrade code paths as root in a container: fresh install yields
640 root pimonitor/ dir750 root pimonitor; re-running on an existing loose-permission config tightens it to the same without altering content; userpimonitorcan read the file whilenobodycannot.bash -n packaging/install.shpasses. Not verified on physical Pi hardware / a live systemd install, but the changed code paths were exercised verbatim.Related Issue
Closes #20
Checklist
go test ./...passes locally) — no Go code changed; the shell-script paths were exercised manually as described above, full Go suite passesgo vet ./...andgolangci-lint runare cleandocs/API.md),configuration (
README.md,packaging/pimonitor.example.yaml), orinstallation/packaging (
packaging/install.sh, systemd units)/api/v1/...response shapes, or a new APIversion was introduced instead