Let an update install host packages, without handing out root
Asked for directly: "später wenn ich etwas lokal erweitere will ich mich nicht auf alle adminseiten einloggen müssen und extra rsync installieren". An update runs as the service account, so anything root-owned on the host is out of its reach and turns into "log into every server once and paste this". The obvious fix — sudo on deploy/install-agent.sh — is not a grant at all: the service account owns the checkout and can rewrite the very script it would be allowed to run as root. So the privileged part is written OUT of the checkout by install-agent.sh, to /usr/local/sbin/clupilot-host-step, root:root 0755, from a quoted here-document. The service account cannot influence a byte of it. sudoers names one exact command line including its argument, so a step added to the helper later is not covered by a grant written before it existed, and the helper refuses anything not on its own list as well. update.sh uses it only when rsync is actually missing, after the restart and never fatally: the archive is collected hours later, and an update that died over a package would be the bigger problem. A helper older than the steps the updater wants is reported through the existing one-time-setup hint — silently doing nothing is worse than an error. Existing servers still need `sudo bash deploy/install-agent.sh` once, because that is the run that puts the helper there. After it, they do not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feat/granted-plans v1.3.3
nexxo
parent
0671e8e119
commit
ef409d1276
|
|
@ -21,26 +21,88 @@ id -u "$APP_USER" >/dev/null 2>&1 || { echo "No such account: $APP_USER" >&2; ex
|
|||
install -o "$APP_USER" -g "$APP_USER" -d "$ROOT/storage/app/deploy"
|
||||
chmod +x "$ROOT/deploy/update-agent.sh"
|
||||
|
||||
# rsync, for the invoice archive.
|
||||
# ── The few things an update needs root for ──────────────────────────────────
|
||||
# An update runs as the service account. Everything root-owned on the host is
|
||||
# therefore out of its reach, and each such thing turns into "log into every
|
||||
# server once and paste this" — which is exactly the work this project exists to
|
||||
# remove.
|
||||
#
|
||||
# It has to be on the HOST, not in the image: a backup that collects the archive
|
||||
# connects by ssh to this machine, and sshd then starts `rsync --server` here.
|
||||
# Without it the pull fails with "command not found" from a NAS whose own setup
|
||||
# is perfectly correct — which is a bad afternoon for whoever is looking.
|
||||
# The obvious fix is to let the service account run THIS script under sudo. That
|
||||
# would be handing it root outright: it owns the checkout, so it can rewrite the
|
||||
# very script it is allowed to run privileged. A grant is only worth anything if
|
||||
# the holder cannot change what it grants.
|
||||
#
|
||||
# Installed from here because this is the entry point that already runs as root,
|
||||
# once per machine, and because the alternative is an operator logging into a
|
||||
# server to install a package by hand every time they set up a new one. rrsync
|
||||
# comes with it, and that is what locks the collecting key to one directory.
|
||||
# So the privileged part is written OUT of the checkout, to a root-owned path,
|
||||
# from a here-document rather than copied — the service account has no way to
|
||||
# influence a single byte of it. What it may then do as root is one fixed
|
||||
# command line, enumerated in sudoers, and nothing else.
|
||||
#
|
||||
# CONTRACT is the version of that fixed set. deploy/update.sh compares it and
|
||||
# says so when this script has to be run again — a new step added here is
|
||||
# useless on a server still carrying the old helper, and silently useless is the
|
||||
# failure mode worth spending a line on.
|
||||
HOST_STEP=/usr/local/sbin/clupilot-host-step
|
||||
|
||||
cat > "$HOST_STEP" <<'EOF'
|
||||
#!/usr/bin/env bash
|
||||
#
|
||||
# The root-owned half of a CluPilot update. Installed by deploy/install-agent.sh
|
||||
# and never read from the application checkout — see the note there.
|
||||
#
|
||||
# One argument, from the list below. Nothing takes free-form input.
|
||||
set -euo pipefail
|
||||
|
||||
CONTRACT=1
|
||||
|
||||
case "${1:-}" in
|
||||
contract)
|
||||
echo "$CONTRACT"
|
||||
;;
|
||||
|
||||
ensure-rsync)
|
||||
# rsync has to be on the HOST: a backup that collects the invoice
|
||||
# archive connects by ssh, and sshd starts `rsync --server` out here,
|
||||
# not in a container. rrsync comes with it, and that is what locks the
|
||||
# collecting key to one directory.
|
||||
command -v rsync >/dev/null 2>&1 && exit 0
|
||||
command -v apt-get >/dev/null 2>&1 || { echo "no apt-get on this host" >&2; exit 3; }
|
||||
export DEBIAN_FRONTEND=noninteractive
|
||||
apt-get update -qq >/dev/null 2>&1 || true
|
||||
apt-get install -y -qq rsync >/dev/null
|
||||
command -v rsync >/dev/null 2>&1 || { echo "rsync still missing after install" >&2; exit 4; }
|
||||
;;
|
||||
|
||||
*)
|
||||
# Refused by name. sudoers already permits only the exact command lines
|
||||
# above, so this is the second of two locks, not the only one.
|
||||
echo "unknown step: ${1:-<none>}" >&2
|
||||
exit 64
|
||||
;;
|
||||
esac
|
||||
EOF
|
||||
|
||||
chown root:root "$HOST_STEP"
|
||||
chmod 0755 "$HOST_STEP"
|
||||
|
||||
# The command line in full, not just the path: sudoers matches arguments too, so
|
||||
# written this way the grant is for `clupilot-host-step ensure-rsync` and for
|
||||
# nothing else the file might ever learn to do.
|
||||
cat > /etc/sudoers.d/clupilot-host-step <<EOF
|
||||
$APP_USER ALL=(root) NOPASSWD: $HOST_STEP ensure-rsync
|
||||
EOF
|
||||
chmod 0440 /etc/sudoers.d/clupilot-host-step
|
||||
visudo -cf /etc/sudoers.d/clupilot-host-step >/dev/null || {
|
||||
echo " ! sudoers fragment rejected — removing it. Updates will ask for rsync by hand."
|
||||
rm -f /etc/sudoers.d/clupilot-host-step
|
||||
}
|
||||
|
||||
# And straight away, through the same helper rather than beside it — one
|
||||
# implementation, exercised on every install instead of only when an update
|
||||
# happens to find rsync missing.
|
||||
if ! command -v rsync >/dev/null 2>&1; then
|
||||
echo " Installing rsync (needed by the invoice archive)"
|
||||
if command -v apt-get >/dev/null 2>&1; then
|
||||
DEBIAN_FRONTEND=noninteractive apt-get update -qq >/dev/null 2>&1 || true
|
||||
DEBIAN_FRONTEND=noninteractive apt-get install -y -qq rsync >/dev/null 2>&1 \
|
||||
|| echo " ! Could not install rsync — the invoice archive cannot be collected until it is there."
|
||||
else
|
||||
echo " ! No apt-get here. Install rsync yourself, or the invoice archive cannot be collected."
|
||||
fi
|
||||
echo " Installing rsync (the invoice archive is collected over ssh)"
|
||||
"$HOST_STEP" ensure-rsync \
|
||||
|| echo " ! Could not install rsync — the invoice archive cannot be collected until it is there."
|
||||
fi
|
||||
|
||||
cat > /etc/systemd/system/clupilot-update-agent.service <<EOF
|
||||
|
|
|
|||
|
|
@ -31,6 +31,11 @@ if [[ $EUID -eq 0 ]]; then
|
|||
exit 1
|
||||
fi
|
||||
STATE_FILE="storage/app/deployed-commit"
|
||||
# The root-owned half of an update, and the version of what it can do. Both are
|
||||
# fixed by deploy/install-agent.sh; this script only asks. Raising HOST_STEP_NEEDS
|
||||
# here is what makes an update tell the operator to run the installer again.
|
||||
HOST_STEP=/usr/local/sbin/clupilot-host-step
|
||||
HOST_STEP_NEEDS=1
|
||||
# Which step is running, for the console to show. Written as a KEY, not as the
|
||||
# sentence below it: the console is translated and this script is not, so an
|
||||
# English line here would surface untranslated in the interface. A run that dies
|
||||
|
|
@ -468,6 +473,25 @@ fi
|
|||
|
||||
reconcile_vpn_readiness
|
||||
|
||||
# ── Host packages the update can install itself ──────────────────────────────
|
||||
# This script cannot install anything on the host: it runs as the service
|
||||
# account. What it can do is ask the root-owned helper that install-agent.sh put
|
||||
# at a path the service account cannot write to, for one fixed command line that
|
||||
# sudoers permits by name. See the long note in deploy/install-agent.sh for why
|
||||
# it is built that way and not as sudo on this script.
|
||||
#
|
||||
# Deliberately after the restart and never fatal. rsync is needed by whoever
|
||||
# collects the invoice archive, some minutes or hours from now — it is not worth
|
||||
# a failed deployment, and an update that stops here would be a far bigger
|
||||
# problem than a backup that has to wait for the next run.
|
||||
if ! command -v rsync >/dev/null 2>&1; then
|
||||
if [[ -x "$HOST_STEP" ]] && sudo -n "$HOST_STEP" ensure-rsync >/dev/null 2>&1; then
|
||||
log "Installed rsync on the host (the invoice archive is collected over ssh)"
|
||||
else
|
||||
warn "rsync is not installed on this host — the invoice archive cannot be collected."
|
||||
fi
|
||||
fi
|
||||
|
||||
phase maintenance_off "Leaving maintenance mode"
|
||||
in_app php artisan up >/dev/null
|
||||
down=0
|
||||
|
|
@ -492,6 +516,20 @@ fi
|
|||
# The proxy's console allowlist has to be wired once, as root, and an update
|
||||
# never runs as root — so an existing installation would keep its hard-coded
|
||||
# list and everything the owner changes in the console would do nothing.
|
||||
# A host helper from before a step was added to it. Nothing is broken, but the
|
||||
# new step silently does nothing — which is worse than an error, because the
|
||||
# next person to look assumes it ran.
|
||||
if [[ -z "$agent_hint" ]]; then
|
||||
# `|| true` on the read, not on the whole test: an installation that has
|
||||
# never had the helper reports 0 and lands in the same hint.
|
||||
host_step_have="$( [[ -x "$HOST_STEP" ]] && "$HOST_STEP" contract 2>/dev/null || true )"
|
||||
[[ "$host_step_have" =~ ^[0-9]+$ ]] || host_step_have=0
|
||||
|
||||
if (( host_step_have < HOST_STEP_NEEDS )); then
|
||||
agent_hint="the update cannot install host packages such as rsync by itself"
|
||||
fi
|
||||
fi
|
||||
|
||||
if [[ -z "$agent_hint" ]] && command -v caddy >/dev/null 2>&1 \
|
||||
&& [[ -f /etc/caddy/Caddyfile ]] \
|
||||
&& ! grep -q 'clupilot-console-allow.conf' /etc/caddy/Caddyfile 2>/dev/null; then
|
||||
|
|
|
|||
|
|
@ -0,0 +1,183 @@
|
|||
<?php
|
||||
|
||||
use Symfony\Component\Process\Process;
|
||||
|
||||
/**
|
||||
* The root-owned half of an update.
|
||||
*
|
||||
* An update runs as the service account, so everything on the host that belongs
|
||||
* to root — installing a package, writing a unit — is out of its reach, and each
|
||||
* such thing turns into "log into every server once and paste this".
|
||||
*
|
||||
* The tempting fix is a sudo grant on deploy/install-agent.sh. That is root, not
|
||||
* a grant: the service account owns the checkout and can rewrite the script it
|
||||
* would be allowed to run privileged. So the privileged part is written OUT of
|
||||
* the checkout to a root-owned path, and the grant names one exact command line.
|
||||
*
|
||||
* These tests run the helper as bash actually runs it — the whole point is what
|
||||
* it REFUSES, and a refusal is an exit code.
|
||||
*/
|
||||
function hostStepScript(): string
|
||||
{
|
||||
$installer = file_get_contents(base_path('deploy/install-agent.sh'));
|
||||
|
||||
// The quoted delimiter matters and is asserted separately below: an
|
||||
// unquoted heredoc would expand the installer's own variables into a file
|
||||
// that is supposed to be fixed at write time.
|
||||
expect($installer)->toContain("cat > \"\$HOST_STEP\" <<'EOF'");
|
||||
|
||||
// betweenFirst, NOT between: `between` cuts at the LAST delimiter, and this
|
||||
// installer has several here-documents after this one — so it handed back
|
||||
// the helper plus everything down to the final EOF, and the extra `EOF`
|
||||
// line then ran as a command. The helper was fine; the measurement was not.
|
||||
$body = Str::betweenFirst($installer, "cat > \"\$HOST_STEP\" <<'EOF'\n", "\nEOF\n");
|
||||
|
||||
// The extraction is part of what these tests trust, so it is checked at
|
||||
// both ends rather than only at the start.
|
||||
expect($body)->toContain('CONTRACT=')
|
||||
->and(rtrim($body))->toEndWith('esac')
|
||||
->and($body)->not->toContain("\nEOF");
|
||||
|
||||
return $body;
|
||||
}
|
||||
|
||||
function runHostStep(string $args, array $onPath = []): Process
|
||||
{
|
||||
$dir = sys_get_temp_dir().'/clupilot-host-step-'.bin2hex(random_bytes(6));
|
||||
mkdir($dir.'/bin', 0755, true);
|
||||
file_put_contents($dir.'/clupilot-host-step', hostStepScript());
|
||||
chmod($dir.'/clupilot-host-step', 0755);
|
||||
|
||||
// A stub apt-get that fails loudly. Any test that does not expect an
|
||||
// install will see it in the output rather than silently tolerating one.
|
||||
file_put_contents($dir.'/bin/apt-get', "#!/bin/sh\necho 'APT-GET WAS CALLED' >&2\nexit 1\n");
|
||||
chmod($dir.'/bin/apt-get', 0755);
|
||||
|
||||
foreach ($onPath as $name => $exit) {
|
||||
file_put_contents($dir."/bin/{$name}", "#!/bin/sh\nexit {$exit}\n");
|
||||
chmod($dir."/bin/{$name}", 0755);
|
||||
}
|
||||
|
||||
$process = Process::fromShellCommandline(
|
||||
'PATH='.escapeshellarg($dir.'/bin').':/usr/bin:/bin '
|
||||
.escapeshellarg($dir.'/clupilot-host-step').' '.$args
|
||||
);
|
||||
$process->run();
|
||||
|
||||
exec('rm -rf '.escapeshellarg($dir));
|
||||
|
||||
return $process;
|
||||
}
|
||||
|
||||
it('refuses a step it does not know by name', function () {
|
||||
// The whole safety argument rests on this. sudoers permits one exact
|
||||
// command line, and this is the second of the two locks: even reached some
|
||||
// other way, the helper does nothing it was not built to do.
|
||||
$process = runHostStep('install-whatever-i-like');
|
||||
|
||||
expect($process->getExitCode())->toBe(64)
|
||||
->and($process->getErrorOutput())->toContain('unknown step')
|
||||
->and($process->getErrorOutput())->not->toContain('APT-GET WAS CALLED');
|
||||
});
|
||||
|
||||
it('refuses being run with no step at all', function () {
|
||||
// `set -u` would otherwise end this as an unbound-variable error, which is
|
||||
// the right outcome by accident and reads as a bug in the helper.
|
||||
$process = runHostStep('');
|
||||
|
||||
expect($process->getExitCode())->toBe(64)
|
||||
->and($process->getErrorOutput())->toContain('unknown step');
|
||||
});
|
||||
|
||||
it('does not run a package manager when rsync is already there', function () {
|
||||
$process = runHostStep('ensure-rsync', ['rsync' => 0]);
|
||||
|
||||
expect($process->getExitCode())->toBe(0)
|
||||
->and($process->getErrorOutput())->not->toContain('APT-GET WAS CALLED');
|
||||
});
|
||||
|
||||
it('says so rather than pretending, on a host with no apt-get', function () {
|
||||
// Nothing here is Debian-only by design, but the install step is. Silence
|
||||
// would leave an operator believing rsync is on a machine where it is not,
|
||||
// and they would find out from a backup that collected nothing.
|
||||
$dir = sys_get_temp_dir().'/clupilot-host-step-'.bin2hex(random_bytes(6));
|
||||
mkdir($dir.'/bin', 0755, true);
|
||||
file_put_contents($dir.'/clupilot-host-step', hostStepScript());
|
||||
chmod($dir.'/clupilot-host-step', 0755);
|
||||
|
||||
// An empty PATH, and bash named outright — the helper's own `#!/usr/bin/env
|
||||
// bash` would not find an interpreter otherwise, and that 127 looks exactly
|
||||
// like the refusal being tested here.
|
||||
$process = new Process(
|
||||
[trim((string) shell_exec('command -v bash')), $dir.'/clupilot-host-step', 'ensure-rsync'],
|
||||
null,
|
||||
['PATH' => $dir.'/bin'],
|
||||
);
|
||||
$process->run();
|
||||
exec('rm -rf '.escapeshellarg($dir));
|
||||
|
||||
expect($process->getExitCode())->toBe(3)
|
||||
->and($process->getErrorOutput())->toContain('no apt-get');
|
||||
});
|
||||
|
||||
it('reports the contract version the updater compares against', function () {
|
||||
$process = runHostStep('contract');
|
||||
|
||||
expect($process->getExitCode())->toBe(0)
|
||||
->and(trim($process->getOutput()))->toMatch('/^\d+$/');
|
||||
|
||||
// And the two halves agree. A helper written by an older installer is the
|
||||
// failure mode this number exists for; if update.sh asked for a version the
|
||||
// current installer never writes, every up-to-date server would be told to
|
||||
// run the installer again forever.
|
||||
$needs = Str::between(file_get_contents(base_path('deploy/update.sh')), 'HOST_STEP_NEEDS=', "\n");
|
||||
|
||||
expect((int) trim($process->getOutput()))->toBe((int) trim($needs));
|
||||
});
|
||||
|
||||
it('grants one command line, not a script the service account can rewrite', function () {
|
||||
$installer = file_get_contents(base_path('deploy/install-agent.sh'));
|
||||
|
||||
expect($installer)
|
||||
// Root's, at a path under root's — the service account cannot edit what
|
||||
// it is allowed to run. This is the entire safety argument.
|
||||
->toContain('HOST_STEP=/usr/local/sbin/clupilot-host-step')
|
||||
->toContain('chown root:root "$HOST_STEP"')
|
||||
->toContain('chmod 0755 "$HOST_STEP"')
|
||||
// Validated before it counts. A malformed fragment in /etc/sudoers.d
|
||||
// breaks sudo for everyone on the machine, not only for this grant.
|
||||
->toContain('visudo -cf /etc/sudoers.d/clupilot-host-step')
|
||||
// Both halves must mean the same path.
|
||||
->and(file_get_contents(base_path('deploy/update.sh')))
|
||||
->toContain('HOST_STEP=/usr/local/sbin/clupilot-host-step');
|
||||
|
||||
// Every sudo grant this installer writes, read as sudoers reads it.
|
||||
preg_match_all('/^\S+\s+ALL=\(root\)\s+NOPASSWD:\s*(.+)$/m', $installer, $m);
|
||||
|
||||
expect($m[1])->not->toBeEmpty();
|
||||
|
||||
foreach ($m[1] as $granted) {
|
||||
// An argument, always. A grant on a bare path permits that path with
|
||||
// ANY arguments — so every step ever added to the helper would be
|
||||
// covered by a decision taken before it existed.
|
||||
expect(trim($granted))->toContain(' ')
|
||||
// And never anything inside the checkout, which the service account
|
||||
// owns: it could rewrite the file and the grant would follow.
|
||||
->not->toContain('$ROOT')
|
||||
->not->toContain('deploy/');
|
||||
}
|
||||
|
||||
expect($m[1])->toContain('$HOST_STEP ensure-rsync');
|
||||
});
|
||||
|
||||
it('never lets a host package stop a deployment', function () {
|
||||
// rsync is wanted by whoever collects the invoice archive, minutes or hours
|
||||
// later. An update that died because a package manager was busy would be a
|
||||
// far bigger problem than a backup waiting for the next run.
|
||||
$update = file_get_contents(base_path('deploy/update.sh'));
|
||||
|
||||
$call = Str::between($update, 'if ! command -v rsync', 'phase maintenance_off');
|
||||
|
||||
expect($call)->toContain('sudo -n "$HOST_STEP" ensure-rsync >/dev/null 2>&1')
|
||||
->and($call)->toContain('warn ');
|
||||
});
|
||||
Loading…
Reference in New Issue