tiny-pilot / tinypilot

Use your Raspberry Pi as a browser-based KVM.
https://tinypilotkvm.com
MIT License
2.89k stars 244 forks source link

Add privilege check to render-template script #1728

Closed jdeanwallace closed 5 months ago

jdeanwallace commented 5 months ago

Resolves https://github.com/tiny-pilot/tinypilot-pro/issues/1190

Please refer to https://github.com/tiny-pilot/tinypilot-pro/issues/1190#issue-2094765706 for PR rationale.

Executing the render-template script with root privileges with now fail. For example, as part of the TinyPilot installation:

$ apt-get install -y ./tinypilot_20240128105411_armhf.deb
...
Preparing to unpack .../tinypilot_20240128105411_armhf.deb ...
Unpacking tinypilot (20240128105411) over (20240128094116) ...
Setting up tinypilot (20240128105411) ...
Warning: The home dir /home/tinypilot you specified already exists.
The system user `tinypilot' already exists. Exiting.
/opt/tinypilot /

This script doesn't require root privileges.
Please re-run as tinypilot:
  runuser tinypilot --command './scripts/render-template'

dpkg: error processing package tinypilot (--configure):
 installed tinypilot package post-installation script subprocess returned error exit status 1
Errors were encountered while processing:
 tinypilot
E: Sub-process /usr/bin/dpkg returned an error code (1)
+ clean_up
+ umount --lazy /mnt/tinypilot-installer
+ rm -rf /opt/tinypilot-updater /mnt/tinypilot-installer

Notes

  1. We had to expand the previously used render-template command into multiple independent commands because the exit code was being ignored/swallowed.
  2. To execute render-template as the tinypilot user, we use the runuser command instead of su. Based on the runuser manual, runuser can only be used by root users which is currently the only way we use render-template:

    The difference between the commands runuser and su is that runuser does not ask for a password (because it may be executed by the root user only) and it uses a different PAM configuration.

Review on CodeApprove

jotaen4tinypilot commented 5 months ago

Fyi, the e2e tests are green again on master.

jdeanwallace commented 5 months ago
Automated comment from CodeApprove ➜

⏳ @jotaen4tinypilot please review this Pull Request