On 11/28/25 08:47, Alyssa Ross wrote:
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)
The checker will delete any temporary files it comes across, so it needs write access. A snapshot is much heavier than a bind mount and isn't automatically cleaned up.
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?
${./download-update} includes the working directory in the Nix store hash, which means that renaming your source tree forces an unnecessary rebuild. builtins.path is the standard way to avoid this.
+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.
Will fix.
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 { … }
I considered it and decided that the extra define in the multisubstitute was cheaper than the extra importas process.
+ 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.
I'd prefer to leave it this way for now. Maybe add a TODO to clean this up.
+} +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!
You're welcome! -- Sincerely, Demi Marie Obenour (she/her/hers)