Skip to main content

max / alloy

Require the font-tool conditional to test the profile Splitting a block on its else says nothing about what was branched on, so a font tool inside any other conditional passed. Ask the condition for $PROFILE as well, and cover it with a fixture block that branches on something else.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-23 20:52 UTC
Signed with PGP, not checked
Commit: a16708ef309513bfc38aac0b7c6bb3428e71beb0
Parent: 03df368
1 file changed, +55 insertions, -0 deletions
@@ -141,6 +141,24 @@
141 141 )
142 142 }
143 143
144 + /// Whether the conditional the branches came from tests `$PROFILE`.
145 + ///
146 + /// [`if_branches`] splits on the `else` and says nothing about what was
147 + /// branched on, so on its own it accepts any conditional at all: a block
148 + /// reading `if [ -d /x ]; then fc-cache; else :; fi` has a first half holding a
149 + /// font tool and passes a check that only looks at the halves. The condition is
150 + /// the part between the block's `if` and its `then`, and it has to name the
151 + /// variable the profiles differ in.
152 + fn tests_the_profile(client: &str) -> bool {
153 + let Some((head, _)) = client.split_once("; then") else {
154 + return false;
155 + };
156 + let Some((_, condition)) = head.rsplit_once("if ") else {
157 + return false;
158 + };
159 + condition.contains("$PROFILE")
160 + }
161 +
144 162 /// The font tools, which all ship in the one package the client installs.
145 163 const FONT_TOOLS: [&str; 3] = ["fc-cache", "fc-list", "fc-match"];
146 164
@@ -324,6 +342,11 @@
324 342 /// would have satisfied. So the branches are split and the halves are asked
325 343 /// separately. The server half may still NAME a tool, in `command -v`, since
326 344 /// proving the tool is unreachable is the whole of what that branch does.
345 + ///
346 + /// Splitting on the `else` says nothing about what was branched on, so the
347 + /// condition is asked for separately ([`tests_the_profile`]): a font tool
348 + /// guarded by anything other than `$PROFILE` runs on whichever builds that
349 + /// other test happens to be true for.
327 350 #[test]
328 351 fn every_font_tool_call_sits_in_the_client_branch() {
329 352 for block in run_blocks(&containerfile()) {
@@ -331,6 +354,11 @@
331 354 continue;
332 355 }
333 356 let (client, server) = if_branches(&block);
357 + assert!(
358 + tests_the_profile(&client),
359 + "this block calls a font tool inside a conditional that does not test \
360 + $PROFILE, so which builds run it is decided by something else:\n{block}",
361 + );
334 362 assert!(
335 363 FONT_TOOLS.iter().any(|tool| client.contains(tool)),
336 364 "this block calls a font tool, and not in the client branch, which is the \
@@ -349,6 +377,33 @@
349 377 }
350 378 }
351 379
380 + /// And the guard fails on the block it is there to catch.
381 + ///
382 + /// The real Containerfile passes every check above, which is also what a check
383 + /// that has stopped looking does. This runs the condition half over blocks
384 + /// written to be wrong.
385 + #[test]
386 + fn a_font_tool_guarded_by_something_other_than_the_profile_is_caught() {
387 + let not_the_profile = "set -eu; if [ -d /usr/share/fonts/quasi ]; then fc-cache -fv; \
388 + else echo nothing; fi";
389 + let (client, _) = if_branches(not_the_profile);
390 + assert!(
391 + client.contains("fc-cache"),
392 + "the fixture puts the tool in the first half, which is what makes it a fixture",
393 + );
394 + assert!(
395 + !tests_the_profile(&client),
396 + "a conditional on a directory is not a conditional on the profile",
397 + );
398 +
399 + let profile = "set -eu; if [ \"$PROFILE\" = client ]; then fc-cache -fv; else echo nothing; fi";
400 + let (client, _) = if_branches(profile);
401 + assert!(
402 + tests_the_profile(&client),
403 + "and the real shape still passes"
404 + );
405 + }
406 +
352 407 /// Every font package the client installs is denied by name in the server
353 408 /// guard.
354 409 ///