From 48ba215f221fdeab4e8e33735bbcbc46879b7c6a Mon Sep 17 00:00:00 2001 From: indaco Date: Sun, 26 Jul 2026 23:59:52 +0200 Subject: [PATCH 1/2] fix(post-install): publish keg-only launchers and retire stale links A keg-only formula that publishes its launcher and completions from post_install installed with nothing on PATH. Those steps name their source with a keg-relative template rather than a base, and the executor did not resolve those tokens, so the path never became absolute and the symlink was refused as relative. The companion remove step was not implemented at all, leaving a stale launcher behind. The templates now resolve against the keg, and remove is a guarded delete: it retires a symlink identified by a substring of its target, so a regular file or a link pointing elsewhere at the same path survives. Reading the link doubles as the type check, and the link is unlinked, never followed. Note the deliberate split between a keg-relative template and a same-named prefix base; both appear in one step when a keg-only formula publishes into the prefix. --- ...-install-steps-keg-templates-and-remove.sh | 95 +++++++++ src/core/post_install_steps.zig | 186 +++++++++++++++++- 2 files changed, 277 insertions(+), 4 deletions(-) create mode 100755 scripts/regressions/post-install-steps-keg-templates-and-remove.sh diff --git a/scripts/regressions/post-install-steps-keg-templates-and-remove.sh b/scripts/regressions/post-install-steps-keg-templates-and-remove.sh new file mode 100755 index 00000000..607f66d5 --- /dev/null +++ b/scripts/regressions/post-install-steps-keg-templates-and-remove.sh @@ -0,0 +1,95 @@ +#!/usr/bin/env bash +# Regression: a keg-only formula that publishes its launcher from +# `post_install_steps` installed with nothing on PATH. +# +# Two gaps, one symptom. The steps name their source with a `{{bin}}`-style +# template rather than a `base`, and those tokens were absent from +# `template_map`, so `expandTemplates` left them literal. The resulting path did +# not start with `/`, `stepSymlink` read that as a relative source and refused +# it, and the companion `remove` step was not implemented at all. Net effect for +# rustup: no `/bin/rustup`, no completions, a stale `rustup-init` left +# behind, and a "post_install partially skipped" warning as the only clue. +# +# The `remove` half is a GUARDED delete — it retires a symlink identified by a +# substring of its target. That guard is the security-relevant part: without it +# the step becomes an unconditional formula-driven delete, so this script +# asserts the guard is still required rather than merely that removal happens. +# +# Nothing drives these steps offline without standing up a live tap, so the +# behaviour is pinned by colocated inline unit tests (`lib_tests`). This script +# asserts the load-bearing code and its tests are present, then builds and runs +# that binary. ~45s, matching its sibling silent-skip-post-install-steps.sh; no +# network. + +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +cd "$ROOT" + +STEPS="$ROOT/src/core/post_install_steps.zig" + +# 1. The keg-relative templates. A formula's `{{bin}}` is inside the keg — not +# `/bin`, which is what the same word means as a `base`. Losing any +# of these puts the token back on the "relative source" refusal path. +for tok in bin bash_completion zsh_completion fish_completion pwsh_completion; do + if ! grep -qE "\.\{ \"$tok\", \." "$STEPS"; then + echo "FAIL: template {{$tok}} is no longer resolved — its steps will be refused as relative" >&2 + exit 1 + fi +done + +# 2. `remove` must be a native step, and `supportedStepType` must agree, or +# migrated formulas get miscounted as needing the Ruby fallback. +if ! grep -qE '\.\{ "remove", \.remove \}' "$STEPS"; then + echo "FAIL: the remove step is no longer registered in step_map" >&2 + exit 1 +fi + +# 3. The guard on remove. An unconditional delete driven by formula data is a +# different and far worse thing than retiring one's own symlink. +if ! grep -q 'symlink_target_contains' "$STEPS"; then + echo "FAIL: remove no longer consults symlink_target_contains — unguarded delete" >&2 + exit 1 +fi +if ! grep -q 'readLinkAbsolute' "$STEPS"; then + echo "FAIL: remove no longer type-checks the path as a symlink before deleting" >&2 + exit 1 +fi + +# 4. Behavioural guards. If a test block is deleted the run below goes green +# vacuously, so assert each is present first. +TEMPLATE_TEST="execute links keg-relative template paths into the prefix" +REMOVE_TEST="execute removes a symlink whose target matches the guard" +KEEP_FILE_TEST="execute leaves a regular file alone on remove" +KEEP_LINK_TEST="execute keeps a symlink whose target does not match the guard" +# The pre-existing base-form path shares stepSymlink; judged too so a fix to +# the template form cannot quietly break the form every other formula uses. +BASE_FORM_TEST="execute runs the filesystem tier natively and leaves the log clean" + +for t in "$TEMPLATE_TEST" "$REMOVE_TEST" "$KEEP_FILE_TEST" "$KEEP_LINK_TEST" "$BASE_FORM_TEST"; do + if ! grep -Rqs -- "$t" "$STEPS"; then + echo "FAIL: guard test missing from post_install_steps.zig: $t" >&2 + exit 1 + fi +done + +if ! 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 + +# One run of the suite, judged per guard line: a pass ends in "OK". +out=$("$ROOT/zig-out/test-bin/lib_tests" 2>&1 || true) +for t in "$TEMPLATE_TEST" "$REMOVE_TEST" "$KEEP_FILE_TEST" "$KEEP_LINK_TEST" "$BASE_FORM_TEST"; do + line=$(printf '%s\n' "$out" | grep -F -- "$t" || true) + if [[ -z "$line" ]]; then + echo "FAIL: guard test did not run: $t" >&2 + exit 1 + fi + if [[ "$line" != *OK ]]; then + echo "FAIL: $t: $line" >&2 + exit 1 + fi +done + +echo "OK: keg-relative templates resolve, remove is guarded, base-form symlinks intact" diff --git a/src/core/post_install_steps.zig b/src/core/post_install_steps.zig index 7469769b..cffb6857 100644 --- a/src/core/post_install_steps.zig +++ b/src/core/post_install_steps.zig @@ -49,6 +49,7 @@ const StepTag = enum { symlink, link_dir, link_children, + remove, compile_gsettings_schemas, gio_querymodules, gdk_pixbuf_query_loaders, @@ -67,6 +68,7 @@ const step_map = std.StaticStringMap(StepTag).initComptime(.{ .{ "symlink", .symlink }, .{ "link_dir", .link_dir }, .{ "link_children", .link_children }, + .{ "remove", .remove }, .{ "compile_gsettings_schemas", .compile_gsettings_schemas }, .{ "gio_querymodules", .gio_querymodules }, .{ "gdk_pixbuf_query_loaders", .gdk_pixbuf_query_loaders }, @@ -159,6 +161,7 @@ fn runStep(ctx: StepsCtx, obj: std.json.ObjectMap) bool { .symlink => stepSymlink(ctx, obj), .link_dir => stepLinkDir(ctx, obj), .link_children => stepLinkChildren(ctx, obj), + .remove => stepRemove(ctx, obj), .compile_gsettings_schemas => runPathTool(ctx, obj, "glib", "glib-compile-schemas", &.{}), .gio_querymodules => runPathTool(ctx, obj, "glib", "gio-querymodules", &.{}), .update_mime_database => runPathTool(ctx, obj, "shared-mime-info", "update-mime-database", &.{}), @@ -192,21 +195,49 @@ pub fn expandTemplates(ctx: StepsCtx, s: []const u8) ![]const u8 { return out.toOwnedSlice(ctx.allocator); } -const template_map = std.StaticStringMap(enum { name, version, version_major, version_major_minor, homebrew_prefix }).initComptime(.{ +/// Note the deliberate split from `base_map` below: a `{{bin}}` **template** +/// is the formula's own `bin` — i.e. inside the keg — whereas a `bin` **base** +/// is `/bin`. Upstream uses the template form to name a source in the +/// keg and the base form to name a target in the prefix; a keg-only formula +/// publishing its launcher uses both in the same step. +const template_map = std.StaticStringMap(enum { + name, + version, + version_major, + version_major_minor, + homebrew_prefix, + keg_bin, + bash_completion, + zsh_completion, + fish_completion, + pwsh_completion, +}).initComptime(.{ .{ "name", .name }, .{ "version", .version }, .{ "version.major", .version_major }, .{ "version.major_minor", .version_major_minor }, .{ "HOMEBREW_PREFIX", .homebrew_prefix }, + .{ "bin", .keg_bin }, + .{ "bash_completion", .bash_completion }, + .{ "zsh_completion", .zsh_completion }, + .{ "fish_completion", .fish_completion }, + .{ "pwsh_completion", .pwsh_completion }, }); fn templateValue(ctx: StepsCtx, token: []const u8) ?[]const u8 { + const a = ctx.allocator; return switch (template_map.get(token) orelse return null) { .name => ctx.name, .version => ctx.version, .version_major => versionComponents(ctx.version, 1), .version_major_minor => versionComponents(ctx.version, 2), .homebrew_prefix => ctx.prefix, + // Keg-relative; the layouts are Homebrew's standard completion dirs. + .keg_bin => std.fmt.allocPrint(a, "{s}/bin", .{ctx.keg_path}) catch null, + .bash_completion => std.fmt.allocPrint(a, "{s}/etc/bash_completion.d", .{ctx.keg_path}) catch null, + .zsh_completion => std.fmt.allocPrint(a, "{s}/share/zsh/site-functions", .{ctx.keg_path}) catch null, + .fish_completion => std.fmt.allocPrint(a, "{s}/share/fish/vendor_completions.d", .{ctx.keg_path}) catch null, + .pwsh_completion => std.fmt.allocPrint(a, "{s}/share/pwsh/completions", .{ctx.keg_path}) catch null, }; } @@ -288,6 +319,12 @@ fn resolvePathSpec(ctx: StepsCtx, obj: std.json.ObjectMap, key: []const u8) ?[]c logUnsupported(ctx, key); return null; }; + return resolveSpec(ctx, spec, key); +} + +/// Resolve one `{base, path, formula}` spec. Split out of `resolvePathSpec` so +/// steps carrying an array of specs can reuse the same base/template handling. +fn resolveSpec(ctx: StepsCtx, spec: std.json.ObjectMap, key: []const u8) ?[]const u8 { const raw = getString(spec, "path") orelse { logUnsupported(ctx, key); return null; @@ -373,8 +410,9 @@ fn stepSymlink(ctx: StepsCtx, obj: std.json.ObjectMap) bool { if (!confined(ctx, target)) return false; const source = resolvePathSpec(ctx, obj, "source") orelse return false; if (source.len == 0 or source[0] != '/') { - // ponytail: relative symlink sources are unused upstream; extend - // when a formula ships one. + // Still refused, but now only for a genuinely relative source. A + // `{{bin}}`-style token used to land here because it survived + // expansion unresolved, which read as relative. logUnsupported(ctx, "symlink with a relative source"); return false; } @@ -384,6 +422,43 @@ fn stepSymlink(ctx: StepsCtx, obj: std.json.ObjectMap) bool { return true; } +/// Retire a symlink this formula planted in an earlier version. +/// +/// Guarded delete, never an unconditional one: the step identifies its own link +/// by a substring of the link's target, so anything else living at that path — +/// a real file, or a link pointing somewhere unrelated — belongs to something +/// else and must survive. `readLinkAbsolute` doubles as the type check; it +/// fails on a regular file, which is exactly the skip we want. The link itself +/// is unlinked, never followed. +fn stepRemove(ctx: StepsCtx, obj: std.json.ObjectMap) bool { + const needle = getString(obj, "symlink_target_contains") orelse { + // Without the guard this would be an unconditional recursive delete + // driven by formula data — not something to infer. + logUnsupported(ctx, "remove without symlink_target_contains"); + return false; + }; + const paths_val = obj.get("paths") orelse { + logUnsupported(ctx, "remove"); + return false; + }; + if (paths_val != .array) { + logUnsupported(ctx, "remove"); + return false; + } + + for (paths_val.array.items) |item| { + if (item != .object) continue; + const path = resolveSpec(ctx, item.object, "paths") orelse continue; + if (!confined(ctx, path)) continue; + + var target_buf: [std.fs.max_path_bytes]u8 = undefined; + const len = std.Io.Dir.readLinkAbsolute(ctx.io, path, &target_buf) catch continue; + if (std.mem.indexOf(u8, target_buf[0..len], needle) == null) continue; + std.Io.Dir.cwd().deleteFile(ctx.io, path) catch {}; + } + return true; +} + fn stepLinkChildren(ctx: StepsCtx, obj: std.json.ObjectMap) bool { const source = resolvePathSpec(ctx, obj, "source") orelse return false; const target = resolvePathSpec(ctx, obj, "target") orelse return false; @@ -901,6 +976,109 @@ test "execute returns false when the formula carries no steps" { try testing.expect(!h.flog.hasErrors()); } +test "execute links keg-relative template paths into the prefix" { + // A keg-only formula can still publish its launcher and completions from + // post_install. Those steps name the source with a `{{bin}}`-style token + // rather than a `base`, so the token has to resolve to a keg path — an + // unexpanded one is not absolute and used to be refused as "relative". + var h = try TestHarness.init(); + defer h.deinit(); + const io = h.io; + const a = h.arena.allocator(); + + const seeds = [_][]const u8{ "bin", "etc/bash_completion.d", "share/zsh/site-functions", "share/fish/vendor_completions.d", "share/pwsh/completions" }; + const leaves = [_][]const u8{ "glow", "glow", "_glow", "glow.fish", "_glow.ps1" }; + for (seeds, leaves) |dir, leaf| { + const d = try std.fmt.allocPrint(a, "{s}/{s}", .{ h.keg, dir }); + try std.Io.Dir.cwd().createDirPath(io, d); + const f = try std.Io.Dir.createFileAbsolute(io, try std.fmt.allocPrint(a, "{s}/{s}", .{ d, leaf }), .{}); + f.close(io); + } + + const json = try testFormulaJson(&h, + \\[{"type":"symlink","force":true,"source":{"path":"{{bin}}/glow"},"target":{"path":"{{HOMEBREW_PREFIX}}/bin/glow"}}, + \\ {"type":"symlink","force":true,"source":{"path":"{{bash_completion}}/glow"},"target":{"path":"{{HOMEBREW_PREFIX}}/etc/bash_completion.d/glow"}}, + \\ {"type":"symlink","force":true,"source":{"path":"{{zsh_completion}}/_glow"},"target":{"path":"{{HOMEBREW_PREFIX}}/share/zsh/site-functions/_glow"}}, + \\ {"type":"symlink","force":true,"source":{"path":"{{fish_completion}}/glow.fish"},"target":{"path":"{{HOMEBREW_PREFIX}}/share/fish/vendor_completions.d/glow.fish"}}, + \\ {"type":"symlink","force":true,"source":{"path":"{{pwsh_completion}}/_glow.ps1"},"target":{"path":"{{HOMEBREW_PREFIX}}/share/pwsh/completions/_glow.ps1"}}] + ); + try testing.expect(execute(h.ctx(), json)); + try testing.expect(!h.flog.hasErrors()); + try testing.expectEqual(@as(usize, 5), h.flog.handled_top_level); + + const links = [_][]const u8{ + try std.fmt.allocPrint(a, "{s}/bin/glow", .{h.prefix}), + try std.fmt.allocPrint(a, "{s}/etc/bash_completion.d/glow", .{h.prefix}), + try std.fmt.allocPrint(a, "{s}/share/zsh/site-functions/_glow", .{h.prefix}), + try std.fmt.allocPrint(a, "{s}/share/fish/vendor_completions.d/glow.fish", .{h.prefix}), + try std.fmt.allocPrint(a, "{s}/share/pwsh/completions/_glow.ps1", .{h.prefix}), + }; + for (links) |p| try std.Io.Dir.accessAbsolute(io, p, .{}); +} + +test "execute removes a symlink whose target matches the guard" { + var h = try TestHarness.init(); + defer h.deinit(); + const io = h.io; + const a = h.arena.allocator(); + + const bin = try std.fmt.allocPrint(a, "{s}/bin", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(io, bin); + const stale = try std.fmt.allocPrint(a, "{s}/glow-init", .{bin}); + const dest = try std.fmt.allocPrint(a, "{s}/bin/glow", .{h.keg}); + try std.Io.Dir.cwd().createDirPath(io, try std.fmt.allocPrint(a, "{s}/bin", .{h.keg})); + (try std.Io.Dir.createFileAbsolute(io, dest, .{})).close(io); + try std.Io.Dir.symLinkAbsolute(io, dest, stale, .{}); + + const json = try testFormulaJson(&h, + \\[{"type":"remove","symlink_target_contains":"Cellar/glow/","paths":[{"path":"{{HOMEBREW_PREFIX}}/bin/glow-init"}]}] + ); + try testing.expect(execute(h.ctx(), json)); + try testing.expect(!h.flog.hasErrors()); + try testing.expectError(error.FileNotFound, std.Io.Dir.accessAbsolute(io, stale, .{})); +} + +test "execute leaves a regular file alone on remove" { + // The guard names a symlink target, so a real file at that path is not the + // thing the step meant to delete. Removing it would destroy user data. + var h = try TestHarness.init(); + defer h.deinit(); + const io = h.io; + const a = h.arena.allocator(); + + const bin = try std.fmt.allocPrint(a, "{s}/bin", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(io, bin); + const real = try std.fmt.allocPrint(a, "{s}/glow-init", .{bin}); + (try std.Io.Dir.createFileAbsolute(io, real, .{})).close(io); + + const json = try testFormulaJson(&h, + \\[{"type":"remove","symlink_target_contains":"Cellar/glow/","paths":[{"path":"{{HOMEBREW_PREFIX}}/bin/glow-init"}]}] + ); + try testing.expect(execute(h.ctx(), json)); + try std.Io.Dir.accessAbsolute(io, real, .{}); +} + +test "execute keeps a symlink whose target does not match the guard" { + var h = try TestHarness.init(); + defer h.deinit(); + const io = h.io; + const a = h.arena.allocator(); + + const bin = try std.fmt.allocPrint(a, "{s}/bin", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(io, bin); + const link = try std.fmt.allocPrint(a, "{s}/glow-init", .{bin}); + const dest = try std.fmt.allocPrint(a, "{s}/etc/elsewhere", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(io, try std.fmt.allocPrint(a, "{s}/etc", .{h.prefix})); + (try std.Io.Dir.createFileAbsolute(io, dest, .{})).close(io); + try std.Io.Dir.symLinkAbsolute(io, dest, link, .{}); + + const json = try testFormulaJson(&h, + \\[{"type":"remove","symlink_target_contains":"Cellar/glow/","paths":[{"path":"{{HOMEBREW_PREFIX}}/bin/glow-init"}]}] + ); + try testing.expect(execute(h.ctx(), json)); + try std.Io.Dir.accessAbsolute(io, link, .{}); +} + test "execute runs the filesystem tier natively and leaves the log clean" { var h = try TestHarness.init(); defer h.deinit(); @@ -1313,7 +1491,7 @@ test "supportedStepType matches the executable tier and rejects the rest" { "mkdir_p", "touch", "write", "symlink", "link_dir", "link_children", "compile_gsettings_schemas", "gio_querymodules", "gdk_pixbuf_query_loaders", "gtk_update_icon_cache", "update_mime_database", "update_desktop_database", - "init_data_dir", + "init_data_dir", "remove", }; for (native) |t| try testing.expect(supportedStepType(t)); const routed = [_][]const u8{ "mkdir", "move", "move_children", "frobnicate" }; From c56d24949e063b2439398b548b24f49176df7f02 Mon Sep 17 00:00:00 2001 From: indaco Date: Mon, 27 Jul 2026 01:01:32 +0200 Subject: [PATCH 2/2] fix(post-install): template config files from declarative steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Formulas that fill in a shipped config from post_install installed with the placeholder still in it — a config naming a user that does not exist, behind an install that reported success. The inreplace step was not implemented, and neither were the etc and user tokens its steps depend on. Because the target is live user configuration, the step refuses rather than guesses and the write is atomic. An unresolved token is never substituted: writing the literal token into a config is worse than declining. There is no regex engine here, so only the anchored whole-line shape upstream uses is honoured, behind a strict metacharacter allowlist — approximating a richer pattern would corrupt the file silently. An unmet guard skips the step instead of inventing configuration. --- ...stall-steps-inreplace-config-templating.sh | 98 +++++ src/core/post_install_steps.zig | 359 +++++++++++++++++- 2 files changed, 443 insertions(+), 14 deletions(-) create mode 100755 scripts/regressions/post-install-steps-inreplace-config-templating.sh diff --git a/scripts/regressions/post-install-steps-inreplace-config-templating.sh b/scripts/regressions/post-install-steps-inreplace-config-templating.sh new file mode 100755 index 00000000..c4717b3a --- /dev/null +++ b/scripts/regressions/post-install-steps-inreplace-config-templating.sh @@ -0,0 +1,98 @@ +#!/usr/bin/env bash +# Regression: formulas that template a config file from `post_install_steps` +# installed with the placeholder still in it. +# +# The `inreplace` step was not implemented, and neither were the `{{etc}}` and +# `{{user}}` tokens its steps depend on. Affected formulas shipped a config +# naming a user that does not exist — `change_this`, `@@HOMEBREW-UNBOUND-USER@@` +# — behind an install that reported success. +# +# The step edits live user configuration, so most of this guards the refusals +# rather than the happy path: +# - an unresolved `{{user}}` must never be written into a config; with no +# USER in the environment the step declines instead; +# - there is no regex engine here. Only the `^.*` shape upstream +# actually uses is honoured, behind a strict metacharacter allowlist; +# anything richer reaches the fallback rather than being approximated, +# because a near-miss silently corrupts the file; +# - an unmet `if_exists` guard skips the step rather than inventing config; +# - the write is atomic, so a crash mid-write cannot truncate the original. +# +# Nothing drives these steps offline without standing up a live tap, so the +# behaviour is pinned by colocated inline unit tests (`lib_tests`). This script +# asserts the load-bearing code and its tests are present, then builds and runs +# that binary. ~45s, matching its siblings; no network. + +set -euo pipefail + +ROOT=$(cd "$(dirname "$0")/../.." && pwd) +cd "$ROOT" + +STEPS="$ROOT/src/core/post_install_steps.zig" + +# 1. The step and the two tokens its steps cannot work without. +if ! grep -qE '\.\{ "inreplace", \.inreplace \}' "$STEPS"; then + echo "FAIL: the inreplace step is no longer registered in step_map" >&2 + exit 1 +fi +for tok in etc user; do + if ! grep -qE "\.\{ \"$tok\", \." "$STEPS"; then + echo "FAIL: template {{$tok}} is no longer resolved" >&2 + exit 1 + fi +done + +# 2. The refusals. Each of these turns a silent config corruption into a loud +# partial skip, so losing one is worse than losing the feature. +if ! grep -q 'unresolved template' "$STEPS"; then + echo "FAIL: inreplace no longer refuses an unresolved template — would write {{user}} into a config" >&2 + exit 1 +fi +if ! grep -q 'fn anchoredLineLiteral' "$STEPS"; then + echo "FAIL: the regexp allowlist is gone — richer patterns would be approximated" >&2 + exit 1 +fi +if ! grep -q 'fn guardsSatisfied' "$STEPS"; then + echo "FAIL: guards are no longer evaluated — inreplace would run on absent config" >&2 + exit 1 +fi +if ! grep -q 'atomicReplaceFile' "$STEPS"; then + echo "FAIL: inreplace no longer writes atomically" >&2 + exit 1 +fi + +# 3. Behavioural guards. A deleted test block would let the run below go green +# vacuously, so assert each is present first. +LITERAL_TEST="execute substitutes a literal inreplace and resolves the invoking user" +GUARD_TEST="execute skips an inreplace whose if_exists guard is unmet" +REGEXP_TEST="execute rewrites a whole line for the anchored regexp form" +RICH_TEST="execute refuses a regexp outside the anchored-line subset" +CLASS_TEST="execute refuses an anchored regexp whose literal hides metacharacters" +NOUSER_TEST="execute refuses an inreplace whose user token cannot be resolved" + +for t in "$LITERAL_TEST" "$GUARD_TEST" "$REGEXP_TEST" "$RICH_TEST" "$CLASS_TEST" "$NOUSER_TEST"; do + if ! grep -Rqs -- "$t" "$STEPS"; then + echo "FAIL: guard test missing from post_install_steps.zig: $t" >&2 + exit 1 + fi +done + +if ! 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 + +out=$("$ROOT/zig-out/test-bin/lib_tests" 2>&1 || true) +for t in "$LITERAL_TEST" "$GUARD_TEST" "$REGEXP_TEST" "$RICH_TEST" "$CLASS_TEST" "$NOUSER_TEST"; do + line=$(printf '%s\n' "$out" | grep -F -- "$t" || true) + if [[ -z "$line" ]]; then + echo "FAIL: guard test did not run: $t" >&2 + exit 1 + fi + if [[ "$line" != *OK ]]; then + echo "FAIL: $t: $line" >&2 + exit 1 + fi +done + +echo "OK: inreplace templates configs, refuses unresolved tokens and unsupported regexps" diff --git a/src/core/post_install_steps.zig b/src/core/post_install_steps.zig index cffb6857..f9a5e112 100644 --- a/src/core/post_install_steps.zig +++ b/src/core/post_install_steps.zig @@ -13,6 +13,8 @@ const std = @import("std"); const sandbox = @import("dsl/sandbox.zig"); const sandbox_macos = @import("sandbox/macos.zig"); const fallback_log = @import("dsl/fallback_log.zig"); +const atomic = @import("../fs/atomic.zig"); +const text_replace = @import("../text_replace.zig"); pub const FallbackLog = fallback_log.FallbackLog; @@ -50,6 +52,7 @@ const StepTag = enum { link_dir, link_children, remove, + inreplace, compile_gsettings_schemas, gio_querymodules, gdk_pixbuf_query_loaders, @@ -69,6 +72,7 @@ const step_map = std.StaticStringMap(StepTag).initComptime(.{ .{ "link_dir", .link_dir }, .{ "link_children", .link_children }, .{ "remove", .remove }, + .{ "inreplace", .inreplace }, .{ "compile_gsettings_schemas", .compile_gsettings_schemas }, .{ "gio_querymodules", .gio_querymodules }, .{ "gdk_pixbuf_query_loaders", .gdk_pixbuf_query_loaders }, @@ -162,6 +166,7 @@ fn runStep(ctx: StepsCtx, obj: std.json.ObjectMap) bool { .link_dir => stepLinkDir(ctx, obj), .link_children => stepLinkChildren(ctx, obj), .remove => stepRemove(ctx, obj), + .inreplace => stepInreplace(ctx, obj), .compile_gsettings_schemas => runPathTool(ctx, obj, "glib", "glib-compile-schemas", &.{}), .gio_querymodules => runPathTool(ctx, obj, "glib", "gio-querymodules", &.{}), .update_mime_database => runPathTool(ctx, obj, "shared-mime-info", "update-mime-database", &.{}), @@ -195,17 +200,17 @@ pub fn expandTemplates(ctx: StepsCtx, s: []const u8) ![]const u8 { return out.toOwnedSlice(ctx.allocator); } -/// Note the deliberate split from `base_map` below: a `{{bin}}` **template** -/// is the formula's own `bin` — i.e. inside the keg — whereas a `bin` **base** -/// is `/bin`. Upstream uses the template form to name a source in the -/// keg and the base form to name a target in the prefix; a keg-only formula -/// publishing its launcher uses both in the same step. +/// Note the split from `base_map` below: the `{{bin}}` *template* is the +/// keg's bin, while the `bin` *base* is `/bin`. A keg-only formula +/// publishing its launcher uses both in one step. const template_map = std.StaticStringMap(enum { name, version, version_major, version_major_minor, homebrew_prefix, + prefix_etc, + user, keg_bin, bash_completion, zsh_completion, @@ -217,6 +222,8 @@ const template_map = std.StaticStringMap(enum { .{ "version.major", .version_major }, .{ "version.major_minor", .version_major_minor }, .{ "HOMEBREW_PREFIX", .homebrew_prefix }, + .{ "etc", .prefix_etc }, + .{ "user", .user }, .{ "bin", .keg_bin }, .{ "bash_completion", .bash_completion }, .{ "zsh_completion", .zsh_completion }, @@ -232,6 +239,10 @@ fn templateValue(ctx: StepsCtx, token: []const u8) ?[]const u8 { .version_major => versionComponents(ctx.version, 1), .version_major_minor => versionComponents(ctx.version, 2), .homebrew_prefix => ctx.prefix, + .prefix_etc => std.fmt.allocPrint(a, "{s}/etc", .{ctx.prefix}) catch null, + // Null when the environment has no USER: the caller must refuse + // rather than write an unresolved token into a live config. + .user => std.process.Environ.getPosix(ctx.environ, "USER"), // Keg-relative; the layouts are Homebrew's standard completion dirs. .keg_bin => std.fmt.allocPrint(a, "{s}/bin", .{ctx.keg_path}) catch null, .bash_completion => std.fmt.allocPrint(a, "{s}/etc/bash_completion.d", .{ctx.keg_path}) catch null, @@ -422,14 +433,117 @@ fn stepSymlink(ctx: StepsCtx, obj: std.json.ObjectMap) bool { return true; } -/// Retire a symlink this formula planted in an earlier version. -/// -/// Guarded delete, never an unconditional one: the step identifies its own link -/// by a substring of the link's target, so anything else living at that path — -/// a real file, or a link pointing somewhere unrelated — belongs to something -/// else and must survive. `readLinkAbsolute` doubles as the type check; it -/// fails on a regular file, which is exactly the skip we want. The link itself -/// is unlinked, never followed. +/// `guards` gate a step on the target's state. Only `if_exists` appears +/// upstream; an unknown condition counts as unmet, so the step is skipped +/// rather than run against an assumption that was never checked. +fn guardsSatisfied(ctx: StepsCtx, obj: std.json.ObjectMap) bool { + const guards = obj.get("guards") orelse return true; + if (guards != .array) return true; + for (guards.array.items) |item| { + if (item != .object) continue; + const raw = getString(item.object, "path") orelse return false; + const path = expandTemplates(ctx, raw) catch return false; + if (!std.mem.eql(u8, getString(item.object, "condition") orelse "", "if_exists")) return false; + std.Io.Dir.accessAbsolute(ctx.io, path, .{}) catch return false; + } + return true; +} + +/// Replace every line beginning with `literal` wholesale. Caller must have +/// verified the pattern fits via `anchoredLineLiteral`. +fn replaceAnchoredLines(ctx: StepsCtx, content: []const u8, literal: []const u8, after: []const u8) ?[]const u8 { + var out: std.ArrayList(u8) = .empty; + var lines = std.mem.splitScalar(u8, content, '\n'); + var first = true; + while (lines.next()) |line| { + if (!first) out.append(ctx.allocator, '\n') catch return null; + first = false; + const repl = if (std.mem.startsWith(u8, line, literal)) after else line; + out.appendSlice(ctx.allocator, repl) catch return null; + } + return out.toOwnedSlice(ctx.allocator) catch null; +} + +/// The literal prefix of a `^.*` pattern — the one regexp shape +/// upstream uses — or null for anything else. There is no regex engine here, +/// so a pattern this cannot fully account for must reach the fallback rather +/// than be approximated against a live config. +fn anchoredLineLiteral(pattern: []const u8) ?[]const u8 { + if (!std.mem.startsWith(u8, pattern, "^") or !std.mem.endsWith(u8, pattern, ".*")) return null; + if (pattern.len < 4) return null; + const literal = pattern[1 .. pattern.len - 2]; + for (literal) |c| switch (c) { + '.', '[', ']', '(', ')', '{', '}', '*', '+', '?', '|', '\\', '^', '$' => return null, + else => {}, + }; + return literal; +} + +/// Substitute inside a config file the formula shipped earlier. The target is +/// live user configuration, so the write is atomic and anything uncertain — +/// unresolved template, empty needle, unsupported regexp — refuses instead. +fn stepInreplace(ctx: StepsCtx, obj: std.json.ObjectMap) bool { + // An unmet guard is the step declining to run, not a failure to report. + if (!guardsSatisfied(ctx, obj)) return true; + + const path = resolvePathSpec(ctx, obj, "path") orelse return false; + if (!confined(ctx, path)) return false; + + const before = getString(obj, "before") orelse { + logUnsupported(ctx, "inreplace"); + return false; + }; + const after_raw = getString(obj, "after") orelse { + logUnsupported(ctx, "inreplace"); + return false; + }; + if (before.len == 0) { + logUnsupported(ctx, "inreplace"); + return false; + } + const after = expandTemplates(ctx, after_raw) catch return false; + if (std.mem.indexOf(u8, after, "{{") != null) { + // Writing a literal `{{user}}` into a config is worse than not running. + logUnsupported(ctx, "inreplace with an unresolved template"); + return false; + } + + const file = std.Io.Dir.openFileAbsolute(ctx.io, path, .{}) catch return false; + const stat = file.stat(ctx.io) catch { + file.close(ctx.io); + return false; + }; + if (stat.size > 4 * 1024 * 1024) { + file.close(ctx.io); + return false; + } + const content = ctx.allocator.alloc(u8, stat.size) catch { + file.close(ctx.io); + return false; + }; + const read = file.readPositionalAll(ctx.io, content, 0) catch { + file.close(ctx.io); + return false; + }; + file.close(ctx.io); + if (read < content.len) return false; + + const updated = if (getFlag(obj, "regexp")) blk: { + const literal = anchoredLineLiteral(before) orelse { + logUnsupported(ctx, "inreplace with an unsupported regexp"); + return false; + }; + break :blk replaceAnchoredLines(ctx, content, literal, after) orelse return false; + } else text_replace.replaceAll(ctx.allocator, content, before, after) catch return false; + + atomic.atomicReplaceFile(ctx.io, path, updated) catch return false; + return true; +} + +/// Retire a symlink this formula planted earlier, identified by a substring of +/// its target. Guarded, never unconditional: a real file or a link pointing +/// elsewhere belongs to something else and must survive. `readLinkAbsolute` +/// doubles as the type check, and the link is unlinked rather than followed. fn stepRemove(ctx: StepsCtx, obj: std.json.ObjectMap) bool { const needle = getString(obj, "symlink_target_contains") orelse { // Without the guard this would be an unconditional recursive delete @@ -848,12 +962,16 @@ fn uniquePrefix(io: std.Io) ![]u8 { return p; } +/// Environment carrying a known `USER`, for steps that template it. +const test_user_environ: std.process.Environ = .{ .block = .{ .slice = &[_:null]?[*:0]const u8{"USER=ada"} } }; + const TestHarness = struct { arena: std.heap.ArenaAllocator, flog: FallbackLog, prefix: []u8, keg: []const u8, io: std.Io, + environ: std.process.Environ = .empty, fn init() !TestHarness { const io = testIo(); @@ -881,6 +999,7 @@ const TestHarness = struct { .prefix = self.prefix, .keg_path = self.keg, .flog = &self.flog, + .environ = self.environ, }; } @@ -976,6 +1095,218 @@ test "execute returns false when the formula carries no steps" { try testing.expect(!h.flog.hasErrors()); } +/// Seed `/etc/` with `body` and return its absolute path. +fn seedEtc(h: *TestHarness, rel: []const u8, body: []const u8) ![]const u8 { + const a = h.arena.allocator(); + const path = try std.fmt.allocPrint(a, "{s}/etc/{s}", .{ h.prefix, rel }); + if (std.fs.path.dirname(path)) |d| try std.Io.Dir.cwd().createDirPath(h.io, d); + const f = try std.Io.Dir.createFileAbsolute(h.io, path, .{}); + try f.writeStreamingAll(h.io, body); + f.close(h.io); + return path; +} + +fn readBack(h: *TestHarness, path: []const u8) ![]const u8 { + const f = try std.Io.Dir.openFileAbsolute(h.io, path, .{}); + defer f.close(h.io); + const st = try f.stat(h.io); + const buf = try h.arena.allocator().alloc(u8, st.size); + _ = try f.readPositionalAll(h.io, buf, 0); + return buf; +} + +test "execute substitutes a literal inreplace and resolves the invoking user" { + // Config templating is the whole point of these steps: the shipped file + // carries a placeholder that only the installing machine can fill in. + var h = try TestHarness.init(); + defer h.deinit(); + h.environ = test_user_environ; + const path = try seedEtc(&h, "glow.conf", "username: \"@@PLACEHOLDER@@\"\n"); + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"glow.conf"}, + \\ "before":"@@PLACEHOLDER@@","after":"{{user}}", + \\ "guards":[{"path":"{{etc}}/glow.conf","condition":"if_exists","id":"1"}]}] + ); + try testing.expect(execute(h.ctx(), json)); + try testing.expect(!h.flog.hasErrors()); + try testing.expectEqualStrings("username: \"ada\"\n", try readBack(&h, path)); +} + +test "execute skips an inreplace whose if_exists guard is unmet" { + // The guard exists because the target is a user config that may never + // have been poured; creating it here would invent configuration. + var h = try TestHarness.init(); + defer h.deinit(); + h.environ = test_user_environ; + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"absent.conf"}, + \\ "before":"x","after":"{{user}}", + \\ "guards":[{"path":"{{etc}}/absent.conf","condition":"if_exists","id":"1"}]}] + ); + try testing.expect(execute(h.ctx(), json)); + try testing.expect(!h.flog.hasErrors()); + const missing = try std.fmt.allocPrint(h.arena.allocator(), "{s}/etc/absent.conf", .{h.prefix}); + try testing.expectError(error.FileNotFound, std.Io.Dir.accessAbsolute(h.io, missing, .{})); +} + +test "execute rewrites a whole line for the anchored regexp form" { + var h = try TestHarness.init(); + defer h.deinit(); + h.environ = test_user_environ; + const path = try seedEtc(&h, "glow.cfg", "keep=1\nglow_user=nobody\nkeep=2\n"); + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"glow.cfg"},"regexp":true, + \\ "before":"^glow_user=.*","after":"glow_user={{user}}", + \\ "guards":[{"path":"{{etc}}/glow.cfg","condition":"if_exists","id":"1"}]}] + ); + try testing.expect(execute(h.ctx(), json)); + try testing.expect(!h.flog.hasErrors()); + try testing.expectEqualStrings("keep=1\nglow_user=ada\nkeep=2\n", try readBack(&h, path)); +} + +test "execute refuses a regexp outside the anchored-line subset" { + // There is no regex engine here. Anything richer must reach the fallback + // rather than be approximated — a near-miss silently corrupts a config. + var h = try TestHarness.init(); + defer h.deinit(); + h.environ = test_user_environ; + const body = "a=1\nb=2\n"; + const path = try seedEtc(&h, "rich.cfg", body); + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"rich.cfg"},"regexp":true, + \\ "before":"^(a|b)=[0-9]+$","after":"z={{user}}"}] + ); + _ = execute(h.ctx(), json); + try testing.expectEqualStrings(body, try readBack(&h, path)); + // Assert the refusal itself, not just an unchanged file: a widened + // allowlist would also leave this content alone, so only the step's own + // outcome distinguishes "declined" from "ran and matched nothing". + try testing.expectEqual(@as(usize, 0), h.flog.handled_top_level); + try testing.expect(h.flog.entries().len > 0); + try testing.expectEqual(fallback_log.FallbackReason.unknown_method, h.flog.entries()[0].reason); +} + +test "execute refuses an anchored regexp whose literal hides metacharacters" { + // `^…​.*` shaped but not literal: the middle is a character class, so the + // prefix match this executor would do is not what the pattern means. Only + // the metacharacter allowlist can catch this — the anchor checks pass. + var h = try TestHarness.init(); + defer h.deinit(); + h.environ = test_user_environ; + const body = "user1=x\nuserA=y\n"; + const path = try seedEtc(&h, "class.cfg", body); + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"class.cfg"},"regexp":true, + \\ "before":"^user[0-9].*","after":"user={{user}}"}] + ); + _ = execute(h.ctx(), json); + try testing.expectEqualStrings(body, try readBack(&h, path)); + // Assert the refusal itself, not just an unchanged file: a widened + // allowlist would also leave this content alone, so only the step's own + // outcome distinguishes "declined" from "ran and matched nothing". + try testing.expectEqual(@as(usize, 0), h.flog.handled_top_level); + try testing.expect(h.flog.entries().len > 0); + try testing.expectEqual(fallback_log.FallbackReason.unknown_method, h.flog.entries()[0].reason); +} + +test "execute refuses an inreplace whose user token cannot be resolved" { + // With no USER in the environment the replacement would write the literal + // token into a live config, which is worse than not running at all. + var h = try TestHarness.init(); + defer h.deinit(); + const body = "username: \"@@PLACEHOLDER@@\"\n"; + const path = try seedEtc(&h, "nouser.conf", body); + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"nouser.conf"}, + \\ "before":"@@PLACEHOLDER@@","after":"{{user}}"}] + ); + _ = execute(h.ctx(), json); + try testing.expectEqualStrings(body, try readBack(&h, path)); +} + +test "anchoredLineLiteral rejects patterns with nothing to anchor on" { + // `^.*` matches every line; treating its empty literal as a prefix would + // rewrite the whole file. + try testing.expect(anchoredLineLiteral("^.*") == null); + try testing.expect(anchoredLineLiteral("user=.*") == null); // unanchored + try testing.expect(anchoredLineLiteral("^user=") == null); // no trailing .* + try testing.expect(anchoredLineLiteral("") == null); + try testing.expectEqualStrings("user=", anchoredLineLiteral("^user=.*").?); +} + +test "replaceAnchoredLines preserves a file with no trailing newline" { + var h = try TestHarness.init(); + defer h.deinit(); + const got = replaceAnchoredLines(h.ctx(), "a=1\nb=2", "b=", "b=9").?; + try testing.expectEqualStrings("a=1\nb=9", got); +} + +test "execute refuses an inreplace with an empty needle" { + // An empty `before` would match everywhere; there is no sane rewrite. + var h = try TestHarness.init(); + defer h.deinit(); + h.environ = test_user_environ; + const body = "keep=1\n"; + const path = try seedEtc(&h, "empty.conf", body); + + const json = try testFormulaJson(&h, + \\[{"type":"inreplace","path":{"base":"etc","path":"empty.conf"},"before":"","after":"x"}] + ); + _ = execute(h.ctx(), json); + try testing.expectEqualStrings(body, try readBack(&h, path)); + try testing.expectEqual(@as(usize, 0), h.flog.handled_top_level); +} + +test "execute refuses a remove that carries no target guard" { + // Without the guard this is an unconditional delete driven by formula data. + var h = try TestHarness.init(); + defer h.deinit(); + const a = h.arena.allocator(); + const bin = try std.fmt.allocPrint(a, "{s}/bin", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(h.io, bin); + const victim = try std.fmt.allocPrint(a, "{s}/keepme", .{bin}); + (try std.Io.Dir.createFileAbsolute(h.io, victim, .{})).close(h.io); + + const json = try testFormulaJson(&h, + \\[{"type":"remove","paths":[{"path":"{{HOMEBREW_PREFIX}}/bin/keepme"}]}] + ); + _ = execute(h.ctx(), json); + try std.Io.Dir.accessAbsolute(h.io, victim, .{}); + try testing.expectEqual(@as(usize, 0), h.flog.handled_top_level); +} + +test "execute refuses to remove a path outside the prefix" { + // Confinement, not the target guard, is what stops a crafted path here. + var h = try TestHarness.init(); + defer h.deinit(); + const a = h.arena.allocator(); + const outside = try std.fmt.allocPrint(a, "{s}-escape", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(h.io, outside); + defer std.Io.Dir.cwd().deleteTree(h.io, outside) catch {}; + const dest = try std.fmt.allocPrint(a, "{s}/Cellar/glow/x", .{h.prefix}); + try std.Io.Dir.cwd().createDirPath(h.io, std.fs.path.dirname(dest).?); + (try std.Io.Dir.createFileAbsolute(h.io, dest, .{})).close(h.io); + const link = try std.fmt.allocPrint(a, "{s}/victim", .{outside}); + try std.Io.Dir.symLinkAbsolute(h.io, dest, link, .{}); + + const json = try std.fmt.allocPrint(a, + \\{{"name":"glow","versions":{{"stable":"1.2.3"}},"post_install_steps":[ + \\ {{"type":"remove","symlink_target_contains":"Cellar/glow/","paths":[{{"base":"absolute","path":"{s}"}}]}}]}} + , .{link}); + _ = execute(h.ctx(), json); + try std.Io.Dir.accessAbsolute(h.io, link, .{}); + // Assert the guard fired rather than the step quietly not resolving: a + // surviving link proves nothing on its own. + try testing.expect(h.flog.entries().len > 0); + try testing.expectEqual(fallback_log.FallbackReason.sandbox_violation, h.flog.entries()[0].reason); +} + test "execute links keg-relative template paths into the prefix" { // A keg-only formula can still publish its launcher and completions from // post_install. Those steps name the source with a `{{bin}}`-style token @@ -1491,7 +1822,7 @@ test "supportedStepType matches the executable tier and rejects the rest" { "mkdir_p", "touch", "write", "symlink", "link_dir", "link_children", "compile_gsettings_schemas", "gio_querymodules", "gdk_pixbuf_query_loaders", "gtk_update_icon_cache", "update_mime_database", "update_desktop_database", - "init_data_dir", "remove", + "init_data_dir", "remove", "inreplace", }; for (native) |t| try testing.expect(supportedStepType(t)); const routed = [_][]const u8{ "mkdir", "move", "move_children", "frobnicate" };