fix(install): make install.sh reliable, and never let root write in the agent folder - #37
Merged
Merged
Conversation
nabil1440
added this pull request to stack #38
September 22, 2026 10:40
This was referenced Sep 22, 2026
nabil1440
force-pushed
the
agent/10-install-sh
branch
from
September 22, 2026 11:03
d862ead to
8c75ead
Compare
… link - Use set -euo pipefail, curl -fsSL and a trap that removes the temporary files. Report the HTTP status of the GitHub API, with a rate-limit message for 403 and 429. Parse tag_name with sed; remove the grep -P step. - Extract only fly-<os>-<arch>, without the owner from the archive, and install it as root:root. Before, the binary in /usr/local/bin belonged to uid 1001 (the CI user), so a local user with that uid could replace a binary that root runs. - On a server with the monitoring agent, /usr/local/bin/fly is a link to ~fly/.fly/bin/fly: install through the link, keep the owner of the target, and restart fly-agent. The CLI and the agent keep one binary. - Install with a rename, so a running fly never sees a partial binary. - make release: the archives hold the binary as root:root (GNU tar and bsdtar). Refs #10
Fixes from the adversarial review of this layer.
- On a server with the monitoring agent, the agent user writes the new
binary (runuser), and root only gives it the file on stdin. Before, root
followed /usr/local/bin/fly into ~fly/.fly/bin: the fly user could point
the binary at /etc/passwd and make root overwrite it, or swap in a link
during the copy and make root give it /etc/shadow.
- Take the user and the binary from fly-agent.service (User=, ExecStart=)
and require <home>/.fly/bin/fly. A server that the old installer split
(a separate /usr/local/bin/fly) gets one binary again, with the link.
- Without the agent, install a root file with a temporary name and a
rename. A link at /usr/local/bin/fly is replaced, never followed, and a
leftover fly.new cannot break the install.
- Restart the agent with try-restart, so a stopped agent stays stopped.
- Read checksums.txt with the same rules as fly update ("*" mark, CRLF).
- Take the first tag_name of the one-line API reply. Add timeouts and
retries to each download. The install command uses curl -fsSL.
Refs #10
nabil1440
force-pushed
the
agent/10-install-sh
branch
from
September 24, 2026 08:09
8c75ead to
d4a1f23
Compare
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
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.
Layer 7 of 7 of the monitoring agent (#31 → #37).
Summary
install.shstops at the first error, with a correct message, and removes its temporary files.Change
install.sh:set -euo pipefail,curl -fsSLwith timeouts and retries, atrap, and nogrep -P./usr/local/binwith a temporary name and a rename. A link there is replaced, not followed.fly-agent.service(User=,ExecStart=). The agent user writes the new binary; root only gives it the file. Then/usr/local/bin/flybecomes the link, and the agent restarts (try-restart).make release: archives with the owner root:root (GNU tar and bsdtar). Before,install.shleft/usr/local/bin/flywith the owner of the CI user (uid 1001).Adversarial review
The first version was refuted. These items are fixed in this PR:
/usr/local/bin/flyinto the folder of the agent user. That user could point the binary at/etc/passwdand make root overwrite it. Now root does not write there./usr/local/bin/fly) gets one binary again.fly.newno longer stops the script without a correct message.fly update.Tests
In Ubuntu 24.04, with the published dev pre-release: a new server, a split server, a link to
/etc/passwd(it stays unchanged), a unit with an unknown binary (stop), a wrong checksum, a dangling link, a leftoverfly.new, and CRLF line ends. shellcheck has no warnings.Closes #10