Re: [PATCH v5 11/13] Support updates via systemd-sysupdate

Alyssa Ross
2025-11-28T13:47:20Z

[-- Attachment #1: Type: text/plain, Size: 10903 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> diff --git a/host/rootfs/Makefile b/host/rootfs/Makefile
> index a6d9f23e9f5277b7c79a53105eb2dfe1bab1451e..74ff64019560aae6387df0e1b3409bc174251bdb 100644
> --- a/host/rootfs/Makefile
> +++ b/host/rootfs/Makefile
> @@ -10,6 +10,7 @@ include file-list.mk
> ROOT_FS = build
>
> DIRS = \
> + boot \
> dev \
> etc/s6-linux-init/env \
> etc/s6-linux-init/run-image/configs \
> @@ -33,13 +34,15 @@ DIRS = \
> etc/s6-linux-init/run-image/vm/by-id \
> etc/s6-linux-init/run-image/vm/by-name \
> ext \
> + home \
> proc \
> run \
> - sys
> + sys \
> + tmp
>
> FIFOS = etc/s6-linux-init/run-image/service/s6-svscan-log/fifo
>
> -BUILD_FILES = build/etc/s6-rc
> +BUILD_FILES = build/etc/s6-rc build/etc/os-release build/etc/update-url
>
> # This rule produces three files but Make only (portably)
> # supports one output per rule. Instead of resorting to temporary
> @@ -59,12 +62,22 @@ $(ROOT_FS_IMAGE): ../../scripts/make-erofs.sh $(PACKAGES_FILE) $(FILES) $(BUILD_
> mkdir -p $(ROOT_FS) && \
> { \
> cat $(PACKAGES_FILE) ;\
> + printf '%s\n%s\n' "$$UPDATE_SIGNING_KEY" /etc/systemd/import-pubring.gpg; \

Inconsistent use of shell variable instead of make macro.

> for file in $(FILES) $(LINKS); do printf '%s\n%s\n' $$file "$${file#image/}"; done ;\
> for file in $(BUILD_FILES); do printf '%s\n%s\n' $$file $${file#build/}; done ;\
> printf 'build/empty\n%s\n' $(DIRS) ;\
> printf 'build/fifo\n%s\n' $(FIFOS) ;\
> } | ../../scripts/make-erofs.sh $@
>
> +build/etc/update-url:
> + mkdir -p build/etc
> + # might have metacharacters, so avoid interpolation
> + printf %s\\n "$${UPDATE_URL:?'update URL empty or missing'}" > build/etc/update-url

I'm learning so many shell parameter expansions I didn't know from you :)

> diff --git a/host/rootfs/image/usr/bin/spectrum-update b/host/rootfs/image/usr/bin/spectrum-update
> new file mode 100755
> index 0000000000000000000000000000000000000000..613b43570d0538fce20296ccb1de2a6364e0df55
> --- /dev/null
> +++ b/host/rootfs/image/usr/bin/spectrum-update
> @@ -0,0 +1,92 @@
> +#!/bin/execlineb -WS1
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +if { mkdir -p -m 0700 /run/updater }
> +
> +# Take a global lock to avoid races.
> +s6-setlock /run/update-lock
> +
> +foreground { redirfd -w 2 /dev/null rmdir -- $1 }
> +if { umask 0077 mkdir -p -- $1 }
> +cd $1
> +foreground {
> + # If this exists already that is okay.
> + foreground { redirfd -w 2 /dev/null btrfs subvolume create -- shared }
> +
> + # Delete any stale temporary files. Delete any existing signature
> + # files. If the VM is still running (it should not be), the VM might
> + # have write access to the directory. However, updates-dir-check is
> + # safe against that.
> + if { updates-dir-check cleanup shared }
> +
> + if {
> + foreground {
> + # TODO: suppress only "subvolume does not exist" errors.
> + redirfd -w 2 /dev/null
> + btrfs subvolume delete snapshot
> + }
> + rm -f snapshot
> + }
> +
> + backtick -E update_vm_id {
> + backtick -E id_path { readlink /run/vm/by-name/sys.appvm-systemd-sysupdate }
> + basename -- $id_path
> + }
> +
> + # $fsdir is read-only to the guest, but read-write to the host.
> + # Directories bind-mounted into it are read-write to the guest.
> + # See etc/s6-linux-init/run-image/service/vhost-user-fs/template/run
> + # for details.
> +

This still refers to a non-existent variable.

> + # Set up /etc with what the VM needs. The VM will overlay this
> + # on its own /etc.
> + #
> + # In the future, this should use a bind mount instead of copying
> + # into a tmpfs. However, this would significantly complicate the
> + # cleanup code. Deleting fs/etc would require undoing the bind
> + # mounts instead of rm -rf. Once this code is in a separate mount
> + # namespace, the copies should be replaced by bind mounts.
> + if {
> + if { rm -rf -- /run/vm/by-id/${update_vm_id}/fs/etc }
> + umask 022
> + if { mkdir -p -- /run/vm/by-id/${update_vm_id}/fs/updates /run/vm/by-id/${update_vm_id}/fs/etc/systemd }
> + if { cp -R -- /etc/vm-sysupdate.d /etc/update-url /run/vm/by-id/${update_vm_id}/fs/etc }
> + cp -- /etc/systemd/import-pubring.gpg /run/vm/by-id/${update_vm_id}/fs/etc/systemd
> + }
> +
> + # If the directory is already mounted, unmount it. This prevents a
> + # confusing error from mount.
> + foreground { redirfd -w 2 /dev/null umount -- /run/vm/by-id/${update_vm_id}/fs/updates }
> +
> + # Share the update directory with the VM.
> + if { mount --bind -- shared /run/vm/by-id/${update_vm_id}/fs/updates }
> +
> + # Start the update VM.
> + if { vm-start $update_vm_id }
> +
> + # Wait for the VM to exit.
> + # TODO: This is racy. If the update finishes before this code runs,
> + # the s6-svwait call will fail.
> + if { s6-svwait -D /run/service/vmm/instance/${update_vm_id} }
> +
> + # Remove the bind mount.
> + if { umount -- /run/vm/by-id/${update_vm_id}/fs/updates }
> +
> + # Ensure that the VM cannot change the directory
> + # while systemd-sysupdate is using it.
> + if { btrfs subvolume snapshot -- shared snapshot }
> +
> + # Validate the update directory. Delete any stale temporary files.
> + # Check that a signature file was downloaded.
> + if { updates-dir-check check snapshot }
> +
> + unshare --mount
> + if { mount --bind -o ro -- snapshot /run/updater }
> +
> + /usr/lib/systemd/systemd-sysupdate update

Why not just make a readonly snapshot?
(btrfs subvolume snapshot -r)

> diff --git a/vm/app/systemd-sysupdate/default.nix b/vm/app/systemd-sysupdate/default.nix
> new file mode 100644
> index 0000000000000000000000000000000000000000..69be0bab500ea2ea6cb3b6d71edbf1a3e7bddbba
> --- /dev/null
> +++ b/vm/app/systemd-sysupdate/default.nix
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: MIT
> +# SPDX-FileCopyrightText: 2023 Alyssa Ross <hi@alyssa.is>
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +
> +import ../../../lib/call-package.nix (
> +{ callSpectrumPackage, curl, lib, src
> +, runCommand, systemd, writeScript
> +}:
> +
> +let
> + downloadUpdate = builtins.path {
> + name = "download-update";
> + path = ./download-update;
> + };

builtins.path is overkill here surely, as opposed to just writing
${./download-update} below?

> +in
> +
> +callSpectrumPackage ../../make-vm.nix {} {
> + providers.net = [ "sys.netvm" ];
> + type = "nix";
> + run = writeScript "run-script" ''
> +#!/usr/bin/env -S execlineb -WS0

#!/bin/execlineb -WS0 would be fine — we know that'll exist in the VM.

> diff --git a/vm/app/systemd-sysupdate/download-update b/vm/app/systemd-sysupdate/download-update
> new file mode 100755
> index 0000000000000000000000000000000000000000..eada41c6c8ad5edcedd9f4d76b76492e0b8be826
> --- /dev/null
> +++ b/vm/app/systemd-sysupdate/download-update
> @@ -0,0 +1,68 @@
> +#!/usr/bin/env -S execlineb -WS0
> +# SPDX-License-Identifier: EUPL-1.2+
> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
> +export LC_ALL C
> +export LANGUAGE C
> +if { mount -toverlay -olowerdir=/run/virtiofs/virtiofs0/etc:/etc -- overlay /etc }
> +backtick tmpdir { mktemp -d /tmp/sysupdate-XXXXXX }
> +# Not a useless use of cat: if there are NUL bytes in the URL
> +# busybox's awk might misbehave.
> +backtick update_url { cat /etc/update-url }
> +if {
> + backtick sed_rhs {
> + # Use awk to both validate the URL and to escape sed metacharacters.
> + # Reject URLs with control characters, query parameters, or fragments.
> + # They *cannot* work and so are rejected to produce better error messages.
> + #
> + # curl rejects control characters with "Malformed input to a URL function".
> + # Fragment specifiers ("#") and query parameters ("?") break concatenating
> + # /SHA256SUMS and /SHA256SUMS.sha256.asc onto the update URL. Also, it is
> + # simpler to reject update URLs that contain whitespace than to try to
> + # escape them.
> + #
> + # Backslash needs to be escaped once for systemd-sysupdate and again for sed.
> + # Ampersand needs to be escaped once for sed.
> + awk "BEGIN {
> + update_url = ENVIRON[\"update_url\"];
> + if (update_url ~ /^[^\\001-\\040?#\\x7F]+$/) {
> + # Use & to avoid extra escaping (16 or 32 backslashes!)
> + # and a divergence between POSIX and GNU awk.
> + gsub(/\\\\/, \"&&&&\", update_url);
> + gsub(/&/, \"\\\\\\\\&\", update_url);
> + print update_url;
> + exit 0;
> + } else {
> + print ARGV[2] > \"/dev/stderr\";
> + exit 100;
> + }
> + }" -- $3
> + "Bad update URL from host: control characters, whitespace, query parameters, and fragment specifiers not allowed"
> + }
> + elglob -w -0 transfer_file_ /etc/vm-sysupdate.d/*.transfer
> + forx -E transfer_file { $transfer_file_ }
> + backtick target_basename {
> + basename -- $transfer_file
> + }
> + multisubstitute {
> + importas -iuS sed_rhs
> + importas -iuS target_basename
> + importas -iuS tmpdir
> + define sed_input $transfer_file
> + }

You could avoid some serial substitution here if you wanted, by not
passing -E to forx:

forx transfer_file { $transfer_file_ }
backtick target_basename {
importas -iuS transfer_file
basename -- $transfer_file
}
multisubstitute {
…
}

> + redirfd -w 1 ${tmpdir}/${target_basename}
> + sed -E -- "s#@UPDATE_URL@#${sed_rhs}#g" $sed_input

Using awk to escape stuff for sed seems a bit Rube Goldberg. Would it
make more sense to just do the replacement in the awk program? Actually
a lot of this might be nicer in awk than execline? Feel free to tell me
to leave it this way for now, though.

> +}
> +multisubstitute {
> + importas -iuS update_url
> + importas -iuS CURL_PATH
> + importas -iuS SYSTEMD_SYSUPDATE_PATH
> + importas -iuS tmpdir
> +}
> +if { $SYSTEMD_SYSUPDATE_PATH --definitions=${tmpdir} update }
> +# [ and ] are allowed in update URLs so that IPv6 addresses work, but
> +# they cause globbing in the curl command-line tool by default. Use --globoff
> +# to disable this feature.
> +if { $CURL_PATH -L --proto-redir =http,https --globoff
> + -o /run/virtiofs/virtiofs0/updates/SHA256SUMS -- ${update_url}/SHA256SUMS }
> +$CURL_PATH -L --proto-redir =http,https --globoff
> + -o /run/virtiofs/virtiofs0/updates/SHA256SUMS.sha256.asc -- ${update_url}/SHA256SUMS.sha256.asc

Much easier to understand now. Thanks!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]