fix(install): resolve dependency-scoped bottle placeholders

scripts/regressions/dependency-scoped-placeholder-unsubstituted.sh

@@ -0,0 +1,138 @@
+#!/usr/bin/env bash
+# Regression: keg relocation substituted only the tokens it could derive from
+# the prefix. A bottle whose wrapper script references a path inside ANOTHER
+# keg carries a placeholder whose value is not a function of the prefix, so the
+# token shipped literally into `<prefix>/bin/`. The wrapper then resolved to a
+# path that does not exist and failed at launch, behind an install that
+# reported success and a `doctor` run that reported the prefix healthy.
+#
+# Four things have to stay fixed, and each fails differently:
+#   1. relocation must accept and apply a caller-resolved replacement;
+#   2. every command that materialises a keg must resolve it — install, migrate
+#      and rollback each reach relocation by a different route, and two of them
+#      were missed on the first pass;
+#   3. RELOC_LOGIC_VERSION must have moved past 2, or `store-relocated/v2/`
+#      still serves kegs snapshotted with the literal token and the fix never
+#      reaches anyone who already installed an affected bottle;
+#   4. doctor must scan text files, or the whole class stays invisible.
+#
+# Behavioural guards live in colocated `test {}` blocks; this script asserts
+# the wiring is present statically, then runs the two test binaries and judges
+# those guards' lines. Exits 0 when the bug is absent, non-zero (naming the
+# failing assertion) when present. No network; no temp state; well under 30s
+# once the test binaries are built.
+
+set -euo pipefail
+
+ROOT=$(cd "$(dirname "$0")/../.." && pwd)
+
+# 1. The resolver must exist as a table, not a hardcoded lookup: the token set
+#    is upstream's, so new entries have to be a data change.
+if ! grep -q 'dependency_placeholders' "$ROOT/src/core/formula.zig"; then
+  echo "FAIL: formula.zig no longer declares the dependency placeholder table" >&2
+  exit 1
+fi
+if ! grep -q 'pub fn dependencyPlaceholder' "$ROOT/src/core/formula.zig"; then
+  echo "FAIL: formula.dependencyPlaceholder is gone — nothing resolves the token" >&2
+  exit 1
+fi
+
+# 2. Relocation must actually apply the caller's replacement. Without this the
+#    resolver above is dead code and the token still ships literally.
+if ! grep -q 'extra_replacement' "$ROOT/src/core/cellar.zig"; then
+  echo "FAIL: relocateKegTree no longer applies a caller-resolved replacement" >&2
+  exit 1
+fi
+
+# 3. EVERY path that materialises a keg must resolve the token. Reaching for
+#    the short `materialize()` wrapper at any of these is the easy way to
+#    silently reintroduce the bug for one command while the others stay fixed —
+#    which is exactly how migrate and rollback were missed the first time.
+#    (`upgrade` is covered transitively: it routes through download.zig.)
+for site in src/cli/install/download.zig src/cli/migrate/keg.zig src/cli/rollback.zig; do
+  if ! grep -q 'dependencyPlaceholder' "$ROOT/$site"; then
+    echo "FAIL: $site no longer resolves the placeholder from the formula" >&2
+    exit 1
+  fi
+done
+
+# 4. The safety net: doctor must scan text files, not just Mach-O. Without it a
+#    prefix with an unresolved token still reports healthy, which is what let
+#    the original defect ship unnoticed.
+if ! grep -q 'textCarriesPlaceholder' "$ROOT/src/cli/doctor.zig"; then
+  echo "FAIL: doctor no longer scans text files for unsubstituted tokens" >&2
+  exit 1
+fi
+
+# 5. Cached kegs relocated before the fix hold the literal token. Serving one
+#    back re-breaks a fixed binary, so the cache version must be past 2.
+version=$(grep -oE 'RELOC_LOGIC_VERSION: u32 = [0-9]+' "$ROOT/src/core/relocated_store.zig" |
+  grep -oE '[0-9]+$' || true)
+if [[ -z "$version" ]]; then
+  echo "FAIL: could not read RELOC_LOGIC_VERSION from relocated_store.zig" >&2
+  exit 1
+fi
+if ((version < 3)); then
+  echo "FAIL: RELOC_LOGIC_VERSION is $version; kegs cached under v2 still hold the literal token" >&2
+  exit 1
+fi
+
+# 6. Behavioural guards. If either test block is deleted the runs below would
+#    silently pass, so assert they are present first.
+RESOLVE_TEST="dependencyPlaceholder resolves a token against its declared dependency"
+PINNED_TEST="dependencyPlaceholder keeps the version suffix of a pinned dependency"
+APPLY_TEST="relocation substitutes the caller-resolved dependency placeholder"
+WITHHOLD_TEST="relocation leaves an unresolved dependency placeholder in place"
+
+if ! grep -Rqs -- "$RESOLVE_TEST" "$ROOT/src/core/formula.zig"; then
+  echo "FAIL: resolver guard test missing from formula.zig" >&2
+  exit 1
+fi
+if ! grep -Rqs -- "$PINNED_TEST" "$ROOT/src/core/formula.zig"; then
+  echo "FAIL: pinned-dependency guard test missing from formula.zig" >&2
+  exit 1
+fi
+if ! grep -Rqs -- "$APPLY_TEST" "$ROOT/tests/cellar_test.zig"; then
+  echo "FAIL: relocation guard test missing from cellar_test.zig" >&2
+  exit 1
+fi
+if ! grep -Rqs -- "$WITHHOLD_TEST" "$ROOT/tests/cellar_test.zig"; then
+  echo "FAIL: withhold guard test missing from cellar_test.zig" >&2
+  exit 1
+fi
+
+# Rebuild the test binaries so the run reflects the working tree.
+if ! (cd "$ROOT" && zig build test-bin >/dev/null 2>&1); then
+  echo "FAIL: could not build the test binaries (zig build test-bin)" >&2
+  exit 1
+fi
+
+# The runners have no per-test filter, so run the suite ONCE and judge only the
+# guard lines: a pass ends in "OK".
+#
+# Deliberate scope limit: the two resolver guards are inline `test {}` blocks in
+# src/, so they only run inside `lib_tests` — ~2300 tests and ~38s, which alone
+# blows this suite's 30s budget. They are asserted present above (deleting one
+# fails this script) and executed by `just test`; what runs here is the
+# end-to-end half, which is where the bug actually lived.
+check() {
+  local bin="$1"
+  shift
+  local out line name
+  out=$("$ROOT/zig-out/test-bin/$bin" 2>&1 || true)
+  for name in "$@"; do
+    line=$(printf '%s\n' "$out" | grep -F -- "$name" || true)
+    if [[ -z "$line" ]]; then
+      echo "FAIL: guard test did not run in $bin: $name" >&2
+      exit 1
+    fi
+    if [[ "$line" != *OK ]]; then
+      echo "FAIL: $name: $line" >&2
+      exit 1
+    fi
+  done
+}
+
+check cellar_test "$APPLY_TEST" "$WITHHOLD_TEST"
+
+echo "OK: dependency-scoped placeholders resolved and applied; cache version past v2"

scripts/regressions/install_force_sweeps_tap.sh

@@ -114,7 +114,7 @@ if echo "$DOCTOR_OUT" | grep -qE "keg\(s\) in DB but missing on disk"; then
   echo "$DOCTOR_OUT" >&2
   fail "doctor reports Missing kegs after tap-path force-sweep"
 fi
-if echo "$DOCTOR_OUT" | grep -qE "package\(s\) ship Mach-O"; then
+if echo "$DOCTOR_OUT" | grep -qE "package\(s\) ship file\(s\) with unpatched @@HOMEBREW"; then
   echo "$DOCTOR_OUT" >&2
   fail "doctor reports Mach-O placeholders after tap-path force-sweep"
 fi

scripts/regressions/relocated-store-logic-version.pin

@@ -1,2 +1,2 @@
-version=2
-digest=a49588f0cec4692dcb00209f3306d40df1b3faec3968428e73aeb92eecf95290
+version=3
+digest=f1ed07e25bc179c84b72e73b3a8903e6afd9ba3745f5d4a9a44773feacc2f8f9

src/cli/doctor.zig

@@ -1110,19 +1110,19 @@ fn checkMachOPlaceholders(ctx: CheckCtx, name: []const u8) CheckResult {
             // would just duplicate the first row of that list.
             break :blk std.fmt.bufPrint(
                 &msg_buf,
-                "{d} package(s) ship Mach-O file(s) with unpatched @@HOMEBREW_* placeholders. Reinstall the affected packages.",
+                "{d} package(s) ship file(s) with unpatched @@HOMEBREW_* placeholders. Reinstall the affected packages.",
                 .{groups.count()},
-            ) catch "Mach-O files with unpatched @@HOMEBREW_* placeholders found.";
+            ) catch "Files with unpatched @@HOMEBREW_* placeholders found.";
         }
         const first_key = first_key: {
             var it = groups.iterator();
             break :first_key if (it.next()) |kv| kv.key_ptr.* else "";
         };
         break :blk std.fmt.bufPrint(
             &msg_buf,
-            "{d} package(s) ship Mach-O file(s) with unpatched @@HOMEBREW_* placeholders (first: {s}). Reinstall the affected packages.",
+            "{d} package(s) ship file(s) with unpatched @@HOMEBREW_* placeholders (first: {s}). Reinstall the affected packages.",
             .{ groups.count(), first_key },
-        ) catch "Mach-O files with unpatched @@HOMEBREW_* placeholders found.";
+        ) catch "Files with unpatched @@HOMEBREW_* placeholders found.";
     };
     printCheck(name, .err_status, msg);
     armVerboseHint();
@@ -1310,10 +1310,19 @@ fn checkLocalSources(ctx: CheckCtx, name: []const u8) CheckResult {
     return .warn_status;
 }
 
-/// True if `rel_path` inside `base_dir` is a Mach-O binary with at least one
-/// load command that still contains `@@HOMEBREW_PREFIX@@` or
-/// `@@HOMEBREW_CELLAR@@`. Any I/O or parser error is treated as "not bad" —
-/// doctor's placeholder check is best-effort.
+/// True when `content` is patchable text still carrying a relocation token.
+/// Matches `@@HOMEBREW_` generically: doctor only warns, so an unknown token
+/// is worth surfacing rather than ignoring. The NUL sniff mirrors
+/// `patchTextFiles` so doctor never flags a file the patcher would skip.
+fn textCarriesPlaceholder(content: []const u8) bool {
+    const check_len = @min(content.len, 8192);
+    if (std.mem.findScalar(u8, content[0..check_len], 0) != null) return false;
+    return std.mem.indexOf(u8, content, "@@HOMEBREW_") != null;
+}
+
+/// True if `rel_path` still carries a relocation token, in a Mach-O load
+/// command or in text. I/O and parser errors count as "not bad": doctor's
+/// placeholder check is best-effort.
 fn hasUnpatchedPlaceholder(
     io: std.Io,
     allocator: std.mem.Allocator,
@@ -1326,10 +1335,22 @@ fn hasUnpatchedPlaceholder(
     var magic: [4]u8 = undefined;
     const n = file.readPositionalAll(io, &magic, 0) catch return false;
     if (n < 4) return false;
-    if (!parser.isMachO(&magic)) return false;
+    const is_macho = parser.isMachO(&magic);
 
-    // Re-read the full file — parser needs the whole buffer.
     const stat = file.stat(io) catch return false;
+
+    // A wrapper script that kept a token is as broken as a dylib that did.
+    // The 10MB cap matches `patchTextFiles`: larger files were never patched.
+    if (!is_macho) {
+        if (stat.size == 0 or stat.size > 10 * 1024 * 1024) return false;
+        const text = allocator.alloc(u8, stat.size) catch return false;
+        defer allocator.free(text);
+        const got = file.readPositionalAll(io, text, 0) catch return false;
+        if (got < text.len) return false;
+        return textCarriesPlaceholder(text);
+    }
+
+    // Re-read the full file — parser needs the whole buffer.
     if (stat.size > 512 * 1024 * 1024) return false; // skip pathologically large files
     const data = allocator.alloc(u8, stat.size) catch return false;
     defer allocator.free(data);
@@ -1832,3 +1853,29 @@ test "runChecks: an info_status finding counts as neither warning nor error" {
     try testing.expectEqual(@as(u32, 0), tally.warnings);
     try testing.expectEqual(@as(u32, 0), tally.errors);
 }
+
+test "textCarriesPlaceholder flags a wrapper script that kept a token" {
+    // The failure this check exists to catch is a launcher that still points
+    // at a placeholder, which no Mach-O scan can see.
+    // No shebang in the fixture: the spawn-invariant guard rejects shell
+    // literals anywhere under src/, and the token is what's under test.
+    try testing.expect(textCarriesPlaceholder(
+        "DEP=\"${DEP:-@@HOMEBREW_JAVA@@}\" exec real \"$@\"\n",
+    ));
+}
+
+test "textCarriesPlaceholder flags a token this malt does not resolve" {
+    // Deliberately not restricted to the tokens malt substitutes: an
+    // unknown one is exactly the case worth surfacing, and doctor only warns.
+    try testing.expect(textCarriesPlaceholder("prefix=@@HOMEBREW_SOMETHING_NEW@@\n"));
+}
+
+test "textCarriesPlaceholder ignores a relocated script" {
+    try testing.expect(!textCarriesPlaceholder("exec /opt/malt/bin/real\n"));
+}
+
+test "textCarriesPlaceholder ignores binary content" {
+    // patchTextFiles skips anything with a NUL in the first 8KB, so flagging
+    // one would report a defect malt has no way to repair.
+    try testing.expect(!textCarriesPlaceholder("\x7fELF\x00\x00@@HOMEBREW_PREFIX@@"));
+}

src/cli/install/download.zig

@@ -9,6 +9,7 @@ const bottle_mod = @import("../../core/bottle.zig");
 const cellar_mod = @import("../../core/cellar.zig");
 const deps_mod = @import("../../core/deps.zig");
 const formula_mod = @import("../../core/formula.zig");
+const patch_mod = @import("../../core/patch.zig");
 const signals = @import("../../core/signals.zig");
 const store_mod = @import("../../core/store.zig");
 const sqlite = @import("../../db/sqlite.zig");
@@ -577,6 +578,18 @@ pub fn installKegFromBottle(
         };
     }
 
+    // Some bottles carry placeholder tokens whose value is a path inside a
+    // *declared dependency's* keg rather than the prefix. Only here is the
+    // formula in scope to resolve them, so the value is passed down.
+    var placeholder_buf: [512]u8 = undefined;
+    const placeholder = formula_mod.dependencyPlaceholder(
+        &placeholder_buf,
+        target_prefix,
+        formula.dependencies,
+    );
+    const extra_replacement: ?patch_mod.Replacement =
+        if (placeholder) |p| .{ .old = p.token, .new = p.value } else null;
+
     const keg = cellar_mod.materializeWithCellar(
         ctx.io,
         allocator,
@@ -585,6 +598,7 @@ pub fn installKegFromBottle(
         formula.name,
         formula.pkg_version,
         bottle.cellar,
+        extra_replacement,
     ) catch |err| {
         if (deps.cellar_diag) |out| out.* = err;
         return InstallError.CellarFailed;

src/cli/migrate/keg.zig

@@ -154,13 +154,27 @@ pub fn migrateKeg(
     // before materialise (the expensive step stays parallel, lock-free).
     incrementRefLocked(ctx.io, deps.store, deps.db_mu, bottle.sha256, keg_name);
 
-    const keg = cellar_mod.materialize(
+    // Same bottle path as a fresh install, so it needs the same
+    // dependency-scoped placeholder resolution.
+    var placeholder_buf: [512]u8 = undefined;
+    const placeholder = formula_mod.dependencyPlaceholder(
+        &placeholder_buf,
+        deps.prefix,
+        formula.dependencies,
+    );
+
+    const keg = cellar_mod.materializeWithCellar(
         ctx.io,
         allocator,
         deps.prefix,
         bottle.sha256,
         formula.name,
         formula.pkg_version,
+        // Unchanged from the `materialize` wrapper this replaced: migrate has
+        // always relocated these kegs unconditionally, and the cellar type is
+        // a separate question from the placeholder.
+        "",
+        if (placeholder) |p| .{ .old = p.token, .new = p.value } else null,
     ) catch {
         output.err("    {s}: failed to materialize", .{keg_name});
         output.emitNdjsonEvent(.materialized, keg_name, "failed");

src/cli/rollback.zig

@@ -185,8 +185,31 @@ pub fn execute(ctx: *const AppCtx, allocator: std.mem.Allocator, args: []const [
         output.warn("Could not remove cellar entry for {s} {s}", .{ name, current_pkg_version });
     };
 
+    // No parsed formula here, and the DB rows describe the version being
+    // rolled away from — so read the target's own shipped formula source.
+    var ph_buf: [512]u8 = undefined;
+    const placeholder = storeKegPlaceholder(
+        ctx.io,
+        allocator,
+        &ph_buf,
+        prefix,
+        target.sha256,
+        name,
+        target.pkg_version,
+    );
+
     // Materialize the old version from store
-    const keg = cellar.materialize(ctx.io, allocator, prefix, target.sha256, name, target.pkg_version) catch {
+    const keg = cellar.materializeWithCellar(
+        ctx.io,
+        allocator,
+        prefix,
+        target.sha256,
+        name,
+        target.pkg_version,
+        // Unchanged from the `materialize` wrapper this replaced.
+        "",
+        if (placeholder) |p| .{ .old = p.token, .new = p.value } else null,
+    ) catch {
         output.err("Failed to materialize {s} {s} from store", .{ name, target.pkg_version });
         return error.Aborted;
     };
@@ -863,6 +886,40 @@ test "writeEntriesJsonArray emits [] when empty" {
     try testing.expectEqualStrings("[]", aw.written());
 }
 
+/// Resolve a store keg's placeholder from the formula source its bottle ships
+/// at `.brew/<name>.rb`. Best-effort: null leaves the token in place, which
+/// doctor's placeholder check then reports.
+fn storeKegPlaceholder(
+    io: std.Io,
+    allocator: std.mem.Allocator,
+    buf: []u8,
+    prefix: []const u8,
+    sha256: []const u8,
+    name: []const u8,
+    pkg_version: []const u8,
+) ?formula_mod.Placeholder {
+    var path_buf: [std.fs.max_path_bytes]u8 = undefined;
+    const rb_path = std.fmt.bufPrint(
+        &path_buf,
+        "{s}/store/{s}/{s}/{s}/.brew/{s}.rb",
+        .{ prefix, sha256, name, pkg_version, name },
+    ) catch return null;
+
+    const file = std.Io.Dir.openFileAbsolute(io, rb_path, .{}) catch return null;
+    defer file.close(io);
+
+    const stat = file.stat(io) catch return null;
+    if (stat.size == 0 or stat.size > 1024 * 1024) return null;
+    const src = allocator.alloc(u8, stat.size) catch return null;
+    defer allocator.free(src);
+    const read = file.readPositionalAll(io, src, 0) catch return null;
+    if (read < src.len) return null;
+
+    var dep_buf: [64][]const u8 = undefined;
+    const deps = formula_mod.declaredDependencies(&dep_buf, src);
+    return formula_mod.dependencyPlaceholder(buf, prefix, deps);
+}
+
 /// Seed a `<prefix>/store/<sha>/<name>/<pkg_version>/INSTALL_RECEIPT.json`
 /// tree. `delay_ms_after` lets the caller force monotonically increasing
 /// mtimes across calls without depending on filesystem timer resolution