From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from atuin.qyliss.net (localhost [IPv6:::1]) by atuin.qyliss.net (Postfix) with ESMTP id 3B198471A; Tue, 25 Nov 2025 18:06:47 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id 0386C4700; Tue, 25 Nov 2025 18:06:45 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-26) on atuin.qyliss.net X-Spam-Level: X-Spam-Status: No, score=-0.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,DMARC_MISSING,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=4.0.1 Received: from fout-a3-smtp.messagingengine.com (fout-a3-smtp.messagingengine.com [103.168.172.146]) by atuin.qyliss.net (Postfix) with ESMTPS id 65EB4467C for ; Tue, 25 Nov 2025 18:06:43 +0000 (UTC) Received: from phl-compute-05.internal (phl-compute-05.internal [10.202.2.45]) by mailfout.phl.internal (Postfix) with ESMTP id B8CDBEC03F3; Tue, 25 Nov 2025 13:06:41 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-05.internal (MEProxy); Tue, 25 Nov 2025 13:06:41 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=alyssa.is; h=cc :cc:content-type:content-type:date:date:from:from:in-reply-to :in-reply-to:message-id:mime-version:references:reply-to:subject :subject:to:to; s=fm2; t=1764094001; x=1764180401; bh=gPSpSdIf3C M0ZDt+fsksuhrgI1yEwDXDTARx2gHSlRA=; b=C4/H6wiEo3d+gZ1jsSNw4nmqnu CPgbe/R6UA7WY+wpWnPrhucCf4XJN14MMwuKww/URlmRl0GpdqTNfc1SsOj7/pDS 05m97V8DsGGIa3N+3SW/YYqC9/Zp/KTVOtomixknqgKz6rU7OxNPeIf0kZVwpy/F m7daXtlRhM24IN9bdSmhqpw3yTvGB8/KMFqDLcWnfw59VvEMGprzv2bKohgxNW7l radMd+XQKTL1iKLkQIjlk8u+kvwMFUraNCT7TeK4LbtAIsUuRvYQMxa5K/GtIZfa L21jq/kTtxzaEUCC77iay5oQ89XA+ejxtcbZWTNwddoishkUTzP/P0n/YFWQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:content-type:date:date :feedback-id:feedback-id:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:subject:subject:to :to:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; t= 1764094001; x=1764180401; bh=gPSpSdIf3CM0ZDt+fsksuhrgI1yEwDXDTAR x2gHSlRA=; b=V2boZ/zPvaPGPJsenqvtgpe5x5FbnfmQkifyTvBiuUBoxPmLr+J 2TRzjNx0dX9Adfsb/CTisMPeIUyuagb0N3jqupySIrCkFZcpQ0uQoMJ456KDBlne VUgDrKrSzVS9ugIwGvCvimdoPcLS4xvtWsQx/VuLayFQ9n6cyOewaMvjbK7P/Hbv KkdVNW3ar6XcIlWjZyKPOb0H07BJtbpgwTCkLdh751yS3+7I8LYnFpFq3Q/xDtY/ j0kBO72H4rSH8rQIIbJocNU2hhzsJ4rm5rTUvNI3wmzbNcEPYFBoKk4Dn7kjeley xHp5DO14kKuXGh8NraH2bdp6k2E4rvjGibg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvgedvudehucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttddtnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpedthfelie eghffgveeuvdehhfffjeehgfdvvefhgfetfffffedvhfdujeelieefleenucffohhmrghi nhepuhgrphhiqdhgrhhouhhprdhorhhgnecuvehluhhsthgvrhfuihiivgeptdenucfrrg hrrghmpehmrghilhhfrhhomhephhhisegrlhihshhsrgdrihhspdhnsggprhgtphhtthho pedvpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopeguvghmihhosggvnhhouhhrse hgmhgrihhlrdgtohhmpdhrtghpthhtohepuggvvhgvlhesshhpvggtthhruhhmqdhoshdr ohhrgh X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 25 Nov 2025 13:06:40 -0500 (EST) Received: by fw12.qyliss.net (Postfix, from userid 1000) id 3DE7A25FE07E; Tue, 25 Nov 2025 19:06:29 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v4 14/14] Validate configuration parameters In-Reply-To: <20251121-updates-v4-14-d4561c42776e@gmail.com> References: <20251121-updates-v4-0-d4561c42776e@gmail.com> <20251121-updates-v4-14-d4561c42776e@gmail.com> Date: Tue, 25 Nov 2025 19:06:28 +0100 Message-ID: <87h5ui0yxn.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: AL3B3Q6EWF7B7GBSABH3MAOWNZFV56LQ X-Message-ID-Hash: AL3B3Q6EWF7B7GBSABH3MAOWNZFV56LQ X-MailFrom: hi@alyssa.is X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-devel.spectrum-os.org-0; header-match-devel.spectrum-os.org-1; header-match-devel.spectrum-os.org-2; header-match-devel.spectrum-os.org-3; header-match-devel.spectrum-os.org-4; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Spectrum OS Development X-Mailman-Version: 3.3.9 Precedence: list List-Id: Patches and low-level development discussion Archived-At: List-Archive: List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Demi Marie Obenour writes: > Wrong values for the version or update URL will cause very confusing > build-time or runtime errors. Provide a better user experience by > validating them up-front. > > The update URL validator is loose. It rejects only URLs that cannot > possibly work: either appending /SHA256SUMS to them doesn't append to > the path, or they will definitely be rejected by curl due to being > malformed. > > The version validator is in lib/config.nix, as the version number is > used in many places. It checks that the version only uses characters > that are permitted by systemd's version number specification [1] and > that will not break code that uses them in shell or sed commands. > > [1]: https://uapi-group.org/specifications/specs/version_format_specifica= tion > > Signed-off-by: Demi Marie Obenour > --- > Changes since v3: > - Validate compression level. > > Changes since v2: > - Use loose URL validation: allow anything that might work. > - Only reject versions that violate the specification. > --- > Documentation/installation/getting-spectrum.adoc | 2 +- > host/rootfs/default.nix | 19 ++++++++++++++++++- > lib/config.nix | 12 +++++++++++- > release/combined/eosimages.nix | 8 +++++++- > 4 files changed, 37 insertions(+), 4 deletions(-) > > diff --git a/Documentation/installation/getting-spectrum.adoc b/Documenta= tion/installation/getting-spectrum.adoc > index 0abc83a9e6fc01084b3faa9b93eb38398b0aef27..919b28f86eddff1b92570d46b= 62a1fbddc32f2d5 100644 > --- a/Documentation/installation/getting-spectrum.adoc > +++ b/Documentation/installation/getting-spectrum.adoc > @@ -93,7 +93,7 @@ you must provide your own. You can do this via > xref:../development/build-configuration.adoc[build configuration]. > The default sets the signing key to `/dev/null` and the server > URL to an invalid value, so updates won't work. To enable updates, > -set `update-url` to the URL of your server and `update-signing-key` > +set `updateUrl` to the URL of your server and `updateSigningKey` Wrong patch surely? > to a binary GnuPG keyring to verify the updates with. Not all possible > URLs will work, but most invalid URLs will cause an error during the > build rather than runtime misbehavior. > diff --git a/host/rootfs/default.nix b/host/rootfs/default.nix > index 1ebaf11cd7e9d61444b6524de6053a0f3cfb82c8..fed99013f960287c3be3941ca= 593b22c55a6f79a 100644 > --- a/host/rootfs/default.nix > +++ b/host/rootfs/default.nix > @@ -85,6 +85,23 @@ let > appvm-systemd-sysupdate =3D callSpectrumPackage ../../vm/app/systemd= -sysupdate {}; > }; >=20=20 > + update-url =3D As a Nix variable, this should stick to camel case rather than introducing another variable naming convention. > + let update-url =3D config.updateUrl; in > + # Use builtins.fromJSON because it supports \uXXXX escapes. > + # This is the same check done by check-url.awk in the update VM. > + # The update code is careful to escape any metacharacters, but some > + # simply cannot be made to work. Concatenating the URL with /SHA256= SUMS > + # must append to the path portion of the URL, and the URL must be one > + # that libcurl will accept. I don't know how Unicode space is handl= ed, > + # but it is a bad idea. > + if builtins.match (builtins.fromJSON "\"^[^\\u0001- #?\\u007F[:space= :]]+$\"" update-url) =3D=3D null then > + builtins.abort '' > + Update URL ${builtins.toJSON update-url} has forbidden character= s. > + Query strings, and fragment specifiers are not supported. > + ASCII control characters and whitespace must be %-encoded. > + '' > + else > + update-url; > packagesSysroot =3D runCommand "packages-sysroot" { > depsBuildBuild =3D [ inkscape ]; > nativeBuildInputs =3D [ xorg.lndir ]; > @@ -152,7 +169,7 @@ stdenvNoCC.mkDerivation { > name =3D "signing-key"; > path =3D config.updateSigningKey; > }; > - UPDATE_URL =3D config.updateUrl; > + UPDATE_URL =3D update-url; > VERSION =3D config.version; > }; >=20=20 > diff --git a/lib/config.nix b/lib/config.nix > index bc5b42f506b7bfd2f66db48610491809351d1a2c..2065be83ad97f8eb011f070d8= c3f3249104d07f4 100644 > --- a/lib/config.nix > +++ b/lib/config.nix > @@ -18,6 +18,16 @@ let > callConfig =3D config: if builtins.typeOf config =3D=3D "lambda" then = config { > inherit default; > } else config; > + finalConfig =3D default // callConfig config; > in >=20=20 > -default // callConfig config > +# Version is used in many files, so validate it here. > +# See https://uapi-group.org/specifications/specs/version_format_specifi= cation > +# for allowed version strings. > +if builtins.match "[[:alnum:]_.~^-]+" finalConfig.version =3D=3D null th= en > + builtins.abort '' > + Version ${builtins.toJSON finalConfig.version} has forbidden charac= ters. > + Only ASCII alphanumerics, ".", "_", "~", "^", "+", and "-" are allo= wed. > + '' > +else > + finalConfig Okay, I suppose that makes sense. In that case all configuration options probably ought to be validated here, in one place. asserts could at least prevent rightwards drift at the expense of error messages, which I think would be fine for quickly catching trivial mistakes as is the goal here. > diff --git a/release/combined/eosimages.nix b/release/combined/eosimages.= nix > index 9cb35dcecee54c17392b609c493272ec83062e9b..5d1e3a67bb81cbb737823bfa3= c75d88f18b31f2a 100644 > --- a/release/combined/eosimages.nix > +++ b/release/combined/eosimages.nix > @@ -4,6 +4,12 @@ > import ../../lib/call-package.nix ( > { callSpectrumPackage, runCommand, e2fsprogs, tar2ext4, config }: >=20=20 > +let > + compressionLevel =3D config.compressionLevel; > +in > +if compressionLevel < 1 || compressionLevel > 9 then > + builtins.abort "Compression level ${builtins.toString compressionLevel= } is invalid (< 1 or > 9)" > +else Even if we keep compressionLevel, this doesn't really have the same justification for validation as the others. A mistake here isn't going to lead to confusing runtime errors. > runCommand "eosimages.img" { > nativeBuildInputs =3D [ e2fsprogs tar2ext4 ]; > imageName =3D "Spectrum-0.0-x86_64-generic.0.Live.img"; > @@ -16,7 +22,7 @@ runCommand "eosimages.img" { > mkdir dir > cd dir > ln -s -- "$image" "$imageName" > - gzip -${builtins.toString (0 + config.compressionLevel)} < "$image" > = "$imageName.gz" > + gzip -${builtins.toString compressionLevel} < "$image" > "$imageName.g= z" > sha256sum -- "$imageName.gz" > "$imageName.gz.sha256" > tar -ch -- "$imageName.gz" "$imageName.gz.sha256" | tar2ext4 -o "$out" > e2label "$out" eosimages > > --=20 > 2.52.0 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCaSXwJAAKCRCZddwkt31p Fa3xAQDaNmZv3ero2x7MC/J/F6dmDS8FuhZWPiWjPe1TKmRSjwEA8L33gme95pu8 ZnZqEJKcnrYEZNT3Lc1Lv9WWV6CpFgo= =0YzP -----END PGP SIGNATURE----- --=-=-=--