diff --git a/scripts/regressions/dependency-scoped-placeholder-unsubstituted.sh b/scripts/regressions/dependency-scoped-placeholder-unsubstituted.sh new file mode 100755 index 00000000..f1935f86 --- /dev/null +++ b/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 `/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" diff --git a/scripts/regressions/install_force_sweeps_tap.sh b/scripts/regressions/install_force_sweeps_tap.sh index 21fb0ba3..def9a7c8 100755 --- a/scripts/regressions/install_force_sweeps_tap.sh +++ b/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 diff --git a/scripts/regressions/relocated-store-logic-version.pin b/scripts/regressions/relocated-store-logic-version.pin index b853807e..48a32c3a 100644 --- a/scripts/regressions/relocated-store-logic-version.pin +++ b/scripts/regressions/relocated-store-logic-version.pin @@ -1,2 +1,2 @@ -version=2 -digest=a49588f0cec4692dcb00209f3306d40df1b3faec3968428e73aeb92eecf95290 +version=3 +digest=f1ed07e25bc179c84b72e73b3a8903e6afd9ba3745f5d4a9a44773feacc2f8f9 diff --git a/src/cli/doctor.zig b/src/cli/doctor.zig index 97ec30a5..15101209 100644 --- a/src/cli/doctor.zig +++ b/src/cli/doctor.zig @@ -1110,9 +1110,9 @@ 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(); @@ -1120,9 +1120,9 @@ fn checkMachOPlaceholders(ctx: CheckCtx, name: []const u8) CheckResult { }; 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@@")); +} diff --git a/src/cli/install/download.zig b/src/cli/install/download.zig index 370840d2..2071a904 100644 --- a/src/cli/install/download.zig +++ b/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; diff --git a/src/cli/migrate/keg.zig b/src/cli/migrate/keg.zig index ccd15312..014809a0 100644 --- a/src/cli/migrate/keg.zig +++ b/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"); diff --git a/src/cli/rollback.zig b/src/cli/rollback.zig index fb5436ef..9d7b157b 100644 --- a/src/cli/rollback.zig +++ b/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/.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 `/store////INSTALL_RECEIPT.json` /// tree. `delay_ms_after` lets the caller force monotonically increasing /// mtimes across calls without depending on filesystem timer resolution diff --git a/src/core/cellar.zig b/src/core/cellar.zig index bb5f9df5..aa698e45 100644 --- a/src/core/cellar.zig +++ b/src/core/cellar.zig @@ -71,13 +71,16 @@ pub fn materialize( name: []const u8, version: []const u8, ) CellarError!Keg { - return materializeWithCellar(io, allocator, prefix, store_sha256, name, version, ""); + return materializeWithCellar(io, allocator, prefix, store_sha256, name, version, "", null); } /// Materialize with an explicit cellar type from the bottle metadata. /// When cellar_type is ":any" or ":any_skip_relocation", Mach-O binary /// patching is skipped (relocatable bottle). Text placeholder substitution /// (@@HOMEBREW_PREFIX@@, @@HOMEBREW_CELLAR@@) always runs. +/// +/// `extra_replacement` carries a substitution the caller resolved from the +/// formula (see `formula.dependencyPlaceholder`); null for most bottles. pub fn materializeWithCellar( io: std.Io, allocator: std.mem.Allocator, @@ -86,6 +89,7 @@ pub fn materializeWithCellar( name: []const u8, version: []const u8, cellar_type: []const u8, + extra_replacement: ?patch.Replacement, ) CellarError!Keg { // Build paths var store_buf: [512]u8 = undefined; @@ -199,7 +203,7 @@ pub fn materializeWithCellar( return CellarError.CloneFailed; }; - try relocateKegTree(io, allocator, cellar_path, cellar_type); + try relocateKegTree(io, allocator, cellar_path, cellar_type, extra_replacement); // Write INSTALL_RECEIPT.json for brew compatibility writeInstallReceipt(io, cellar_path, name, version, store_sha256); @@ -376,11 +380,14 @@ pub fn cellarLinksPath(io: std.Io, allocator: std.mem.Allocator, cellar_path: [] /// the local-Cellar copy fallback (`materializeFromLocalCellar`) so /// every keg lands on disk with byte-identical relocation regardless /// of whether it came from the store or a sibling brew install. +/// `extra_replacement` is one caller-resolved substitution applied after the +/// prefix-derived ones, for tokens this module cannot derive on its own. fn relocateKegTree( io: std.Io, allocator: std.mem.Allocator, cellar_path: []const u8, cellar_type: []const u8, + extra_replacement: ?patch.Replacement, ) CellarError!void { const new_prefix = atomic.maltPrefixOrAbort(); @@ -428,13 +435,19 @@ fn relocateKegTree( else => return CellarError.PatchFailed, }; - const text_replacements = [_]patch.Replacement{ - .{ .old = "@@HOMEBREW_PREFIX@@", .new = new_prefix }, - .{ .old = "@@HOMEBREW_CELLAR@@", .new = new_cellar }, - .{ .old = "/opt/homebrew", .new = new_prefix }, - .{ .old = "/usr/local", .new = new_prefix }, - }; - _ = patch.patchTextFiles(io, allocator, cellar_path, &text_replacements) catch |e| { + // Last on purpose: `patchTextFiles` runs sequential passes, so appending + // keeps the substituted path out of the prefix rewrites above. + var text_reps_buf: [5]patch.Replacement = undefined; + text_reps_buf[0] = .{ .old = "@@HOMEBREW_PREFIX@@", .new = new_prefix }; + text_reps_buf[1] = .{ .old = "@@HOMEBREW_CELLAR@@", .new = new_cellar }; + text_reps_buf[2] = .{ .old = "/opt/homebrew", .new = new_prefix }; + text_reps_buf[3] = .{ .old = "/usr/local", .new = new_prefix }; + var text_reps_len: usize = 4; + if (extra_replacement) |r| { + text_reps_buf[4] = r; + text_reps_len = 5; + } + _ = patch.patchTextFiles(io, allocator, cellar_path, text_reps_buf[0..text_reps_len]) catch |e| { std.log.warn("text patching failed for {s}: {s}", .{ cellar_path, @errorName(e) }); }; @@ -597,7 +610,10 @@ pub fn materializeFromLocalCellar( return CellarError.CloneFailed; }; - try relocateKegTree(io, allocator, cellar_path, cellar_type); + // No extra replacement: this keg is copied from a sibling brew install, + // which already resolved every dependency-scoped token at its own + // install time. + try relocateKegTree(io, allocator, cellar_path, cellar_type, null); // Tap-aware receipt: the source-of-truth tap is the sibling // brew install's, not "homebrew/core". `mt list` and friends use diff --git a/src/core/formula.zig b/src/core/formula.zig index f5a66da3..d61b567a 100644 --- a/src/core/formula.zig +++ b/src/core/formula.zig @@ -60,6 +60,79 @@ pub fn pkgVersion(buf: []u8, version: []const u8, revision: i64) ![]const u8 { return std.fmt.bufPrint(buf, "{s}_{d}", .{ version, revision }); } +/// A bottle placeholder whose value is a path inside one of the formula's +/// *dependencies*, resolved against the live prefix. +pub const Placeholder = struct { + token: []const u8, + value: []const u8, +}; + +/// Tokens whose value lives in a dependency's keg. A table because the token +/// set is upstream's: adding one is a data change. +const dependency_placeholders = [_]struct { + token: []const u8, + dep: []const u8, + subpath: []const u8, +}{ + .{ + .token = "@@HOMEBREW_JAVA@@", + .dep = "openjdk", + .subpath = "libexec/openjdk.jdk/Contents/Home", + }, +}; + +/// Runtime dependency names from a keg's own `.brew/.rb`, filled into +/// `out`. Callers holding a parsed `Formula` should use its `dependencies`; +/// this is for paths without one, where the DB rows describe a different +/// version than the keg on disk. +pub fn declaredDependencies(out: [][]const u8, rb_source: []const u8) []const []const u8 { + const decl = "depends_on "; + var n: usize = 0; + var lines = std.mem.tokenizeScalar(u8, rb_source, '\n'); + while (lines.next()) |line| { + if (n == out.len) break; + const trimmed = std.mem.trim(u8, line, " \t\r"); + if (!std.mem.startsWith(u8, trimmed, decl)) continue; + + // Only the quoted form names a formula; `depends_on :xcode` and + // friends are platform predicates, not kegs. + const rest = trimmed[decl.len..]; + if (rest.len == 0 or rest[0] != '"') continue; + const close = std.mem.indexOfScalarPos(u8, rest, 1, '"') orelse continue; + if (std.mem.indexOf(u8, rest[close..], ":build") != null) continue; + + out[n] = rest[1..close]; + n += 1; + } + return out[0..n]; +} + +/// Resolve this formula's dependency-scoped placeholder into `buf`. +/// Null when it declares no matching dependency: guessing would bake a path +/// to a keg the formula never asked for. +pub fn dependencyPlaceholder( + buf: []u8, + prefix: []const u8, + dependencies: []const []const u8, +) ?Placeholder { + for (dependency_placeholders) |entry| { + for (dependencies) |dep| { + const pinned = dep.len > entry.dep.len and + std.mem.startsWith(u8, dep, entry.dep) and + dep[entry.dep.len] == '@'; + if (!std.mem.eql(u8, dep, entry.dep) and !pinned) continue; + + const value = std.fmt.bufPrint( + buf, + "{s}/opt/{s}/{s}", + .{ prefix, dep, entry.subpath }, + ) catch return null; + return .{ .token = entry.token, .value = value }; + } + } + return null; +} + /// Parsed Homebrew formula. Every `[]const u8` and `[]const []const u8` /// field is owned by `_parsed` (either borrowed from the JSON source /// buffer or allocated through the parse arena); valid only until @@ -795,3 +868,122 @@ test "parseFormula releases every auxiliary allocation through deinit" { try testing.expect(formula.service != null); try testing.expect(formula.bottle_files != null); } + +test "dependencyPlaceholder resolves a token against its declared dependency" { + // Wrappers that carry the token are unrunnable until it is substituted, + // and the value is a path into the dependency's keg, not the prefix. + var buf: [256]u8 = undefined; + const deps = [_][]const u8{ "openjdk", "ca-certificates" }; + const got = dependencyPlaceholder(&buf, "/opt/malt", &deps).?; + try testing.expectEqualStrings("@@HOMEBREW_JAVA@@", got.token); + try testing.expectEqualStrings( + "/opt/malt/opt/openjdk/libexec/openjdk.jdk/Contents/Home", + got.value, + ); +} + +test "dependencyPlaceholder keeps the version suffix of a pinned dependency" { + // A pinned variant installs under its own opt/ name; folding it onto the + // unpinned one would point the wrapper at a keg the formula didn't ask for. + var buf: [256]u8 = undefined; + const deps = [_][]const u8{"openjdk@21"}; + const got = dependencyPlaceholder(&buf, "/opt/malt", &deps).?; + try testing.expectEqualStrings( + "/opt/malt/opt/openjdk@21/libexec/openjdk.jdk/Contents/Home", + got.value, + ); +} + +test "dependencyPlaceholder declines a formula with no matching dependency" { + // Most bottles carry no such token; substituting a guessed value would + // be worse than leaving them alone. + var buf: [256]u8 = undefined; + const deps = [_][]const u8{ "pcre2", "zstd" }; + try testing.expect(dependencyPlaceholder(&buf, "/opt/malt", &deps) == null); +} + +test "declaredDependencies reads the names a keg's own formula source declares" { + var out: [8][]const u8 = undefined; + const src = + \\class Benerator < Formula + \\ desc "x" + \\ depends_on "openjdk@11" + \\ depends_on "zstd" + \\end + ; + const got = declaredDependencies(&out, src); + try testing.expectEqual(@as(usize, 2), got.len); + try testing.expectEqualStrings("openjdk@11", got[0]); + try testing.expectEqualStrings("zstd", got[1]); +} + +test "declaredDependencies skips build-only dependencies" { + // Parity with the API `dependencies` field the install path uses: a + // build-only jdk is not present at runtime, so resolving a token against + // it would point at a keg that need not exist. + var out: [8][]const u8 = undefined; + const got = declaredDependencies(&out, + \\ depends_on "cmake" => :build + \\ depends_on "openjdk" + ); + try testing.expectEqual(@as(usize, 1), got.len); + try testing.expectEqualStrings("openjdk", got[0]); +} + +test "declaredDependencies ignores commented and symbol forms" { + var out: [8][]const u8 = undefined; + const got = declaredDependencies(&out, + \\ # depends_on "ghost" + \\ depends_on :xcode + \\ depends_on "real" + ); + try testing.expectEqual(@as(usize, 1), got.len); + try testing.expectEqualStrings("real", got[0]); +} + +test "declaredDependencies stops at the caller's capacity" { + var out: [1][]const u8 = undefined; + const got = declaredDependencies(&out, + \\ depends_on "a" + \\ depends_on "b" + ); + try testing.expectEqual(@as(usize, 1), got.len); +} + +test "declaredDependencies ignores an unterminated quote" { + // A truncated or hand-edited .rb must yield nothing rather than a slice + // running past the closing quote that isn't there. + var out: [4][]const u8 = undefined; + try testing.expectEqual(@as(usize, 0), declaredDependencies(&out, " depends_on \"openjdk\n").len); +} + +test "declaredDependencies handles a source with no dependencies" { + var out: [4][]const u8 = undefined; + try testing.expectEqual(@as(usize, 0), declaredDependencies(&out, "class X < Formula\nend\n").len); +} + +test "declaredDependencies tolerates a zero-capacity buffer" { + var out: [0][]const u8 = undefined; + try testing.expectEqual(@as(usize, 0), declaredDependencies(&out, " depends_on \"openjdk\"\n").len); +} + +test "dependencyPlaceholder declines an empty dependency list" { + var buf: [256]u8 = undefined; + try testing.expect(dependencyPlaceholder(&buf, "/opt/malt", &.{}) == null); +} + +test "dependencyPlaceholder declines rather than truncate a too-small buffer" { + // A truncated path would point somewhere real but wrong; refusing leaves + // the token visible instead, which doctor then reports. + var buf: [8]u8 = undefined; + const deps = [_][]const u8{"openjdk"}; + try testing.expect(dependencyPlaceholder(&buf, "/opt/malt", &deps) == null); +} + +test "dependencyPlaceholder does not mistake a lookalike dependency name" { + // Only the exact name or an `@`-qualified variant counts — a longer name + // that merely starts with it is a different formula. + var buf: [256]u8 = undefined; + const deps = [_][]const u8{"openjdk-doc"}; + try testing.expect(dependencyPlaceholder(&buf, "/opt/malt", &deps) == null); +} diff --git a/src/core/relocated_store.zig b/src/core/relocated_store.zig index e3050d88..88b83fa2 100644 --- a/src/core/relocated_store.zig +++ b/src/core/relocated_store.zig @@ -38,7 +38,11 @@ pub const RelocatedStoreError = error{ /// v2: the Mach-O patcher learned to drop LC_RPATHs that relocation collapses /// onto one prefix; v1 entries were snapshotted before that and ship the /// duplicate that makes dyld abort at launch. -pub const RELOC_LOGIC_VERSION: u32 = 2; +/// +/// v3: relocation gained dependency-scoped placeholder substitution. v2 entries +/// cached the literal token, so an affected wrapper restored from one stays +/// broken however many times it is reinstalled. +pub const RELOC_LOGIC_VERSION: u32 = 3; /// Reject anything that is not exactly 64 lowercase hex characters. /// Run this before forming any path so traversal sequences (`..`, `/`) and diff --git a/tests/cellar_test.zig b/tests/cellar_test.zig index c24ea8ed..fd6aab18 100644 --- a/tests/cellar_test.zig +++ b/tests/cellar_test.zig @@ -73,6 +73,28 @@ fn createBottleFixture(allocator: std.mem.Allocator, prefix: []const u8, sha: [] } } +/// A keg with one `bin/` wrapper carrying a dependency-scoped placeholder — +/// a token whose value is a path inside another keg, so relocation cannot +/// derive it from the prefix alone. +fn createPlaceholderBottleFixture(allocator: std.mem.Allocator, prefix: []const u8, sha: []const u8, name: []const u8, ver_dir: []const u8) !void { + const keg = try std.fmt.allocPrint(allocator, "{s}/store/{s}/{s}/{s}", .{ prefix, sha, name, ver_dir }); + defer allocator.free(keg); + try test_io.cwd().createDirPath(std.Options.debug_io, keg); + + const bin_dir = try std.fmt.allocPrint(allocator, "{s}/bin", .{keg}); + defer allocator.free(bin_dir); + try test_io.makeDirAbsolute(std.Options.debug_io, bin_dir); + + const script_path = try std.fmt.allocPrint(allocator, "{s}/bin/wrapper", .{keg}); + defer allocator.free(script_path); + const f = try test_io.createFileAbsolute(std.Options.debug_io, script_path, .{}); + try f.writeStreamingAll( + std.Options.debug_io, + "#!/bin/bash\nDEP_HOME=\"${DEP_HOME:-@@HOMEBREW_JAVA@@}\" exec @@HOMEBREW_CELLAR@@/pkg/1.0/libexec/bin/wrapper \"$@\"\n", + ); + f.close(std.Options.debug_io); +} + fn setupMaltDirs(allocator: std.mem.Allocator, prefix: []const u8) !void { const dirs = [_][]const u8{ "store", "Cellar", "opt", "bin", "lib" }; for (dirs) |d| { @@ -116,6 +138,7 @@ test "materialize handles version with revision suffix" { "pcre2", "10.47", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -177,6 +200,7 @@ test "materialize replaces a pre-existing Cellar/{name}/{version} directory (gh# "lld@21", "21.1.8_1", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -210,6 +234,7 @@ test "materialize handles exact version match (no revision)" { "jq", "1.7.1", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -242,7 +267,8 @@ test "placeholder substitution runs for relocatable bottles" { "rel123", "stow", "2.4.1", - ":any", // relocatable — the bug scenario + ":any", // relocatable — the bug scenario, + null, ); defer testing.allocator.free(keg.path); @@ -259,6 +285,86 @@ test "placeholder substitution runs for relocatable bottles" { try testing.expect(std.mem.indexOf(u8, content, prefix) != null); } +test "relocation substitutes the caller-resolved dependency placeholder" { + // Without this the token survives into the installed wrapper, which then + // resolves to a path that does not exist and fails at launch. + const prefix = try createTestDir(testing.allocator); + defer { + test_io.deleteTreeAbsolute(std.Options.debug_io, prefix) catch {}; + testing.allocator.free(prefix); + } + + try setupMaltDirs(testing.allocator, prefix); + try createPlaceholderBottleFixture(testing.allocator, prefix, "dep123", "pkg", "1.0"); + + const old_env = setMaltPrefix(prefix); + defer restoreMaltPrefix(old_env); + + var value_buf: [512]u8 = undefined; + const value = try std.fmt.bufPrint( + &value_buf, + "{s}/opt/openjdk/libexec/openjdk.jdk/Contents/Home", + .{prefix}, + ); + + const keg = try cellar_mod.materializeWithCellar( + std.Options.debug_io, + testing.allocator, + prefix, + "dep123", + "pkg", + "1.0", + ":any", + .{ .old = "@@HOMEBREW_JAVA@@", .new = value }, + ); + defer testing.allocator.free(keg.path); + + var script_buf: [512]u8 = undefined; + const script_path = try std.fmt.bufPrint(&script_buf, "{s}/bin/wrapper", .{keg.path}); + const content = try readFile(testing.allocator, script_path); + defer testing.allocator.free(content); + + try testing.expect(std.mem.indexOf(u8, content, "@@HOMEBREW_JAVA@@") == null); + try testing.expect(std.mem.indexOf(u8, content, value) != null); +} + +test "relocation leaves an unresolved dependency placeholder in place" { + // Substituting a guessed value would point the wrapper at a keg the + // formula never asked for — worse than leaving the token visible. + const prefix = try createTestDir(testing.allocator); + defer { + test_io.deleteTreeAbsolute(std.Options.debug_io, prefix) catch {}; + testing.allocator.free(prefix); + } + + try setupMaltDirs(testing.allocator, prefix); + try createPlaceholderBottleFixture(testing.allocator, prefix, "dep456", "pkg", "1.0"); + + const old_env = setMaltPrefix(prefix); + defer restoreMaltPrefix(old_env); + + const keg = try cellar_mod.materializeWithCellar( + std.Options.debug_io, + testing.allocator, + prefix, + "dep456", + "pkg", + "1.0", + ":any", + null, + ); + defer testing.allocator.free(keg.path); + + var script_buf: [512]u8 = undefined; + const script_path = try std.fmt.bufPrint(&script_buf, "{s}/bin/wrapper", .{keg.path}); + const content = try readFile(testing.allocator, script_path); + defer testing.allocator.free(content); + + try testing.expect(std.mem.indexOf(u8, content, "@@HOMEBREW_JAVA@@") != null); + // The prefix-derived tokens still relocate — only the extra one is withheld. + try testing.expect(std.mem.indexOf(u8, content, "@@HOMEBREW_CELLAR@@") == null); +} + test "placeholder substitution replaces multiple tokens in single file" { const prefix = try createTestDir(testing.allocator); defer { @@ -280,6 +386,7 @@ test "placeholder substitution replaces multiple tokens in single file" { "pkg", "1.0", "", + null, ); defer testing.allocator.free(keg.path); @@ -332,6 +439,7 @@ test "files with no placeholders are left unchanged" { "noop", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -379,6 +487,7 @@ test "binary files are skipped by text patching without error" { "binpkg", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -446,6 +555,7 @@ test "materializeWithCellar short-circuits when the relocated cache has the sha" "cached", "1.0", ":any", + null, ); testing.allocator.free(keg_pre.path); try relocated_mod.save(std.Options.debug_io, testing.allocator, prefix, valid_test_sha, "cached", "1.0"); @@ -464,6 +574,7 @@ test "materializeWithCellar short-circuits when the relocated cache has the sha" "cached", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -498,6 +609,7 @@ test "materializeWithCellar populates the relocated cache after a cold install" "snap", "0.1", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -574,6 +686,7 @@ test "materializeWithCellar rebuilds a cached keg whose binary would abort dyld" "poisoned", "1.0", ":any", + null, ); testing.allocator.free(first.path); try testing.expect(relocated_mod.has(std.Options.debug_io, prefix, valid_test_sha)); @@ -606,6 +719,7 @@ test "materializeWithCellar rebuilds a cached keg whose binary would abort dyld" "poisoned", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -636,6 +750,7 @@ test "materializeWithCellar trusts a snapshot it already verified" { "trusted", "1.0", ":any", + null, ); testing.allocator.free(first.path); try testing.expect(relocated_mod.isVerified(std.Options.debug_io, prefix, valid_test_sha)); @@ -661,6 +776,7 @@ test "materializeWithCellar trusts a snapshot it already verified" { "trusted", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); @@ -706,6 +822,7 @@ test "materializeWithCellar refuses a keg whose binary kept an unsubstituted pla "unpatched", "2.0", ":any", + null, )); // A keg that cannot load is not left installed, and never gets cached. @@ -763,6 +880,7 @@ test "failed materialize cleans up empty Cellar/{name}/ parent dir" { "ghost", "0.0.1", ":any", + null, ); try testing.expectError(cellar_mod.CellarError.CloneFailed, result); @@ -802,6 +920,7 @@ test "failed materialize leaves sibling versions untouched" { "keeper", "2.0", ":any", + null, ); try testing.expectError(cellar_mod.CellarError.CloneFailed, result); @@ -964,6 +1083,7 @@ test "materialize rewrites @@HOMEBREW_PREFIX@@ in Mach-O rpath for :any bottle" name, version, ":any", + null, ); defer testing.allocator.free(keg.path); @@ -1147,6 +1267,7 @@ test "P9: materialize patches @@HOMEBREW_PREFIX@@ in EVERY fat-binary arch slice name, version, ":any", + null, ); defer testing.allocator.free(keg.path); @@ -1238,6 +1359,7 @@ test "materialize rewrites @@HOMEBREW_CELLAR@@ in Mach-O rpath for :any bottle" name, version, ":any_skip_relocation", + null, ); defer testing.allocator.free(keg.path); @@ -1430,6 +1552,7 @@ test "warm cache-hit reinstall re-pours a wiped overlay config" { "fc", "1.0", ":any", + null, ); testing.allocator.free(keg_cold.path); try testing.expect(relocated_mod.has(std.Options.debug_io, prefix, valid_test_sha)); @@ -1457,6 +1580,7 @@ test "warm cache-hit reinstall re-pours a wiped overlay config" { "fc", "1.0", ":any", + null, ); testing.allocator.free(keg_warm.path); diff --git a/tests/install_warm_path_test.zig b/tests/install_warm_path_test.zig index ae1bba90..6f90bcd2 100644 --- a/tests/install_warm_path_test.zig +++ b/tests/install_warm_path_test.zig @@ -111,6 +111,7 @@ test "install → uninstall → reinstall takes the relocated cache short-circui "warmpkg", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); } @@ -138,6 +139,7 @@ test "install → uninstall → reinstall takes the relocated cache short-circui "warmpkg", "1.0", ":any", + null, ); defer testing.allocator.free(keg.path); try testing.expect(pathExists(keg.path)); @@ -173,6 +175,7 @@ test "non-APFS-style cache miss still allows a successful pipeline reinstall" { "nocache", "0.1", ":any", + null, ); defer testing.allocator.free(keg.path); } @@ -195,6 +198,7 @@ test "non-APFS-style cache miss still allows a successful pipeline reinstall" { "nocache", "0.1", ":any", + null, ); defer testing.allocator.free(keg.path); // Pipeline rebuilt the keg AND restored the cache — warm reinstalls