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 651E020462; Thu, 13 Nov 2025 17:17:09 +0000 (UTC) Received: by atuin.qyliss.net (Postfix, from userid 993) id DC05F2045D; Thu, 13 Nov 2025 17:17:06 +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 fhigh-a5-smtp.messagingengine.com (fhigh-a5-smtp.messagingengine.com [103.168.172.156]) by atuin.qyliss.net (Postfix) with ESMTPS id B628B2045B for ; Thu, 13 Nov 2025 17:17:02 +0000 (UTC) Received: from phl-compute-01.internal (phl-compute-01.internal [10.202.2.41]) by mailfhigh.phl.internal (Postfix) with ESMTP id E5C6714001C8; Thu, 13 Nov 2025 12:17:00 -0500 (EST) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Thu, 13 Nov 2025 12:17:00 -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=1763054220; x=1763140620; bh=GZSLfqQgVs 8IwL2KVgEGagINoM7xX6GMglcgJHJFWFU=; b=hubPNmcCUoXTZRJ3rBZQ0zQUQZ c7aJMRIi26VlOJpny7+pus9pTMTL/48lFudTywQMaHnokj4uSkf38K/RKjlDR45C q6vmBKg+JNbDwGS9kVAhvjO3wGsb1Tzg0ty/Xhm5By3P+ivfs7XuMME+Xb3BRkoU ts9lPlQemPFcrEvxgCEuyNUT8IiWGiJVSImhX6KP4my5Vsr1XvIG4M3jIgwnHGiv TL1XaG+fv7sBThPcOu4K/yq/6JgTRgEUXsuwWTYjIxL0yZdBGuHp8OwwEHHckP4t lGIFW6Z0GXh5EqqHnyzp2IWh3WULjLSNdA6sZ4pcdq5e+7RJw7b70vjAtSkg== 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= 1763054220; x=1763140620; bh=GZSLfqQgVs8IwL2KVgEGagINoM7xX6GMglc gJHJFWFU=; b=hmvenCffJjCuQn/e1B53UdQncNXQTFoJQR+HhIGkBF/wNcxJJ+F T2qUptwFERb6o2IH+7L599FkSjRzjLZMj52tRYu0Hzkgf/khn8ev/wbGmtMkXEnP /c+mQYRZvYUgqZrhHZ7QmMzueXcUNHi9eAK13uhNYfjdUjVYIv40TcynU2HPMZeP mXLdV850g0FhZmR+vEiIk6RJSgnrBQQtHeNUBAoPZRI5jAzPjRWRhPO8Yf1kqBIM PoKVO3FMIlYE1vOFdaN4LJldS7vLpzf4Gu5LNPKMvI6aJYJh+/vU1keHQizw+CgO EZLzecnC9uaLt6GIxjJpIMpmr0OANy75TkA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdeggddvtdejheduucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhephffvvefujghffffkgggtsehgtderredttddtnecuhfhrohhmpeetlhihshhsrgcu tfhoshhsuceohhhisegrlhihshhsrgdrihhsqeenucggtffrrghtthgvrhhnpeeiudffue eilefgtefgtddttdekkeehkefgheekudefveetgeefiefftedvteeuveenucevlhhushht vghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehhihesrghlhihsshgrrd hishdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpdhrtghpthhtohep uggvmhhiohgsvghnohhurhesghhmrghilhdrtghomhdprhgtphhtthhopeguvghvvghlse hsphgvtghtrhhumhdqohhsrdhorhhg X-ME-Proxy: Feedback-ID: i12284293:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 13 Nov 2025 12:17:00 -0500 (EST) Received: by fw12.qyliss.net (Postfix, from userid 1000) id 7388A139D81E; Thu, 13 Nov 2025 18:16:59 +0100 (CET) From: Alyssa Ross To: Demi Marie Obenour Subject: Re: [PATCH v2 8/8] lib/config.nix: Validate configuration parameters In-Reply-To: <20251112-updates-v2-8-88d96bf81b79@gmail.com> References: <20251112-updates-v2-0-88d96bf81b79@gmail.com> <20251112-updates-v2-8-88d96bf81b79@gmail.com> Date: Thu, 13 Nov 2025 18:16:57 +0100 Message-ID: <87jyztc0om.fsf@alyssa.is> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Message-ID-Hash: GHLYQLNHG5FXOP2LCRMLRXSA27J5ZBTR X-Message-ID-Hash: GHLYQLNHG5FXOP2LCRMLRXSA27J5ZBTR 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. > > Signed-off-by: Demi Marie Obenour > --- > lib/config.nix | 41 +++++++++++++++++++++++++++++++++++------ > 1 file changed, 35 insertions(+), 6 deletions(-) > > diff --git a/lib/config.nix b/lib/config.nix > index 5b6b95013734202b7e2e01d5ffce313080658006..660a2427447fd9851e60e955d= a6bd1a5d71cfdac 100644 > --- a/lib/config.nix > +++ b/lib/config.nix > @@ -19,10 +19,39 @@ let > inherit default; > } else config; > finalConfig =3D default // callConfig config; > + > + # Only allow unreserved characters, : (for port numbers), /, and %-enc= oding. > + # The rest of the code is allowed to assume that these are the only ch= aracters > + # in the update URL. > + # Do not use [:alnum:] or [:hexdigit:] as they depend on the locale in= POSIX. Did we confirm that's true for Nix? It would be a big problem if it was. > + # Query strings and fragment identifiers break appending > + # /SHA256SUMS and /SHA256SUMS.gpg to a URL. > + # [, ], {, and } would cause globbing in curl. > + url-regex =3D "^https?://([ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopq= rstuvwxyz0123456789:./~-]|%[ABCDEFabcdef0123456789]{2})+$"; > + update-url =3D finalConfig.update-url; > + > + # Only allow a numeric version for now. > + number_re =3D "(0|[1-9][0-9]{0,2})"; > + version_re =3D "^(${number_re}\\.){2}${number_re}$"; Why? Since this is only to make developers' lives a bit easier, we should err on the side of allowing stuff through rather than restricting what might be a valid use case. > in > - finalConfig // { > - update-signing-key =3D builtins.path { > - name =3D "signing-key"; > - path =3D finalConfig.update-signing-key; > - }; > - } > + if !builtins.isString update-url then > + builtins.abort "Update URL must be a string, not ${builtins.typeOf u= pdate-url}" > + else if builtins.match "^https?://.*" update-url =3D=3D null then > + builtins.abort "Update URL ${builtins.toJSON update-url} has unsuppo= rted scheme (not https:// or http://) or is invalid" > + else if builtins.match url-regex update-url =3D=3D null then > + builtins.abort "Update URL ${builtins.toJSON update-url} has forbidd= en characters" > + else if builtins.substring (builtins.stringLength update-url - 1) 1 up= date-url =3D=3D "/" then > + builtins.abort "Update URL ${builtins.toJSON update-url} must not en= d with /" > + else if !builtins.isString finalConfig.version then > + builtins.abort "Version must be a string, not ${builtins.typeOf fina= lConfig.version}" > + else if builtins.match version_re finalConfig.version =3D=3D null then > + builtins.abort "Version ${builtins.toJSON finalConfig.version} is in= valid" > + else if !builtins.isPath finalConfig.update-signing-key then > + builtins.abort "Update verification key file is of type ${builtins.t= ypeOf finalConfig.update-signing-key}, not path" I wouldn't bother checking the types. There'll be a decent error message if the wrong type is used anyway, and I don't want to get into checking the type of the other config values. I wonder if it would be more maintainable to have this more local, where the values are actually used. That way, it's more likely they'll be kept up to date. > + else > + finalConfig // { > + update-signing-key =3D builtins.path { > + name =3D "signing-key"; > + path =3D finalConfig.update-signing-key; > + }; > + } > > --=20 > 2.51.2 --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iHUEARYKAB0WIQQGoGac7QfI+H5ZtFCZddwkt31pFQUCaRYSigAKCRCZddwkt31p FShVAP9gjUIyUitu++5Qe8DRSWWzIGe6FubeHXf+XyrXVDp54wD+Lzo2LD/UDVNe gNNi5tEJPDfmQm09y8WUVTtt9+rXMA8= =N4tD -----END PGP SIGNATURE----- --=-=-=--