From 2c6090490563f91101d8aab8229c56034942f661 Mon Sep 17 00:00:00 2001 From: indaco Date: Thu, 7 May 2026 18:17:26 +0200 Subject: [PATCH] fix(core/ruby_subprocess): split TapNotFound into actionable failure modes Operators triaging a post_install failure now see one of TapNotFound, FetchFailed, or PostInstallBodyNotFound depending on which arm fired, instead of every miss collapsing onto TapNotFound. A hash mismatch is no longer filed as "no local tap". --- src/core/ruby_subprocess.zig | 164 ++++++++++++++++++++++++++------- tests/ruby_subprocess_test.zig | 56 +++++++++++ 2 files changed, 189 insertions(+), 31 deletions(-) diff --git a/src/core/ruby_subprocess.zig b/src/core/ruby_subprocess.zig index 1ffed18..8c37c28 100644 --- a/src/core/ruby_subprocess.zig +++ b/src/core/ruby_subprocess.zig @@ -17,6 +17,7 @@ pub const RubyError = error{ TapNotFound, FormulaSourceNotFound, PostInstallBodyNotFound, + FetchFailed, ScriptWriteFailed, PostInstallFailed, OutOfMemory, @@ -28,9 +29,10 @@ pub const RubyError = error{ pub fn describeError(err: RubyError) []const u8 { return switch (err) { RubyError.RubyNotFound => "no Ruby interpreter found (tried /opt/homebrew, /usr/local, rbenv, asdf, PATH)", - RubyError.TapNotFound => "post_install source unavailable: no local homebrew-core tap and the hash-pinned GitHub fallback did not produce a body (network failure, formula not in the pinned manifest, or hash mismatch)", - RubyError.FormulaSourceNotFound => "formula .rb source not found in the homebrew-core tap", - RubyError.PostInstallBodyNotFound => "could not extract post_install body from formula source", + RubyError.TapNotFound => "no local homebrew-core tap clone, and the hash-pinned GitHub fallback was not viable for this formula", + RubyError.FormulaSourceNotFound => "formula .rb source not found in the homebrew-core tap, and the hash-pinned GitHub fallback could not recover it", + RubyError.PostInstallBodyNotFound => "could not extract a post_install body from the formula source", + RubyError.FetchFailed => "hash-pinned GitHub fallback fetch failed (network error, HTTP status, hash mismatch, or formula absent from the pinned manifest)", RubyError.ScriptWriteFailed => "could not write the temporary Ruby wrapper script", RubyError.PostInstallFailed => "post_install script failed to run or exited non-zero", RubyError.OutOfMemory => "out of memory running post_install", @@ -125,49 +127,78 @@ pub fn resolveFormulaRbPath(io: std.Io, buf: *[1024]u8, tap_path: []const u8, na return new_path; } -/// Fetch a formula's .rb source from the pinned homebrew-core commit, -/// verify its SHA256 against the embedded manifest, and extract the -/// post_install body. -/// -/// This path is the fallback when the homebrew-core tap is not cloned -/// locally. It refuses anything whose hash isn't pre-authorized in -/// `pins_manifest.txt` — a floating-HEAD fetch would give an attacker -/// who controls the raw.githubusercontent.com response (MITM with a -/// valid cert, compromised CDN edge, branch rewrite) a direct path to -/// code execution via `--use-system-ruby`. -/// -/// Returns the post_install body or null on any fetch / verify / parse -/// failure. All failures are silent-to-caller; the caller decides -/// whether to warn. -pub fn fetchPostInstallFromGitHub(io: std.Io, environ: std.process.Environ, allocator: std.mem.Allocator, name: []const u8) ?[]const u8 { +/// Outcome of the GitHub fallback fetch. Distinguishing `body_not_found` +/// (we got source, no parseable post_install) from `fetch_failed` +/// (network, HTTP, hash, or manifest miss) is what lets the resolver +/// surface an actionable tag instead of collapsing onto TapNotFound. +const FetchOutcome = union(enum) { + body: []const u8, + body_not_found, + fetch_failed, +}; + +/// Tagged variant of `fetchPostInstallFromGitHub` that preserves the +/// distinction between a fetch arm that failed and one that succeeded +/// but yielded no extractable body. Internal: the public optional-slice +/// wrapper is what CLI call sites consume. +fn fetchPostInstallFromGitHubTagged( + io: std.Io, + environ: std.process.Environ, + allocator: std.mem.Allocator, + name: []const u8, +) FetchOutcome { // Reject anything that wouldn't pass the API name allowlist // ([a-z0-9@._+-]) — the URL path substitutes the name directly. - api_mod.validateName(name) catch return null; + api_mod.validateName(name) catch return .fetch_failed; // Fail-closed: no manifest entry, no fetch. The caller decides whether // to warn — core stays headless. - const expected_hash = pins.expectedSha256(name) orelse return null; + const expected_hash = pins.expectedSha256(name) orelse return .fetch_failed; var url_buf: [512]u8 = undefined; const url = std.fmt.bufPrint( &url_buf, "https://raw.githubusercontent.com/Homebrew/homebrew-core/{s}/Formula/{c}/{s}.rb", .{ &pins.homebrew_core_commit_sha, name[0], name }, - ) catch return null; + ) catch return .fetch_failed; var http = http_client.HttpClient.init(io, environ, allocator); defer http.deinit(); - var resp = http.get(url) catch return null; + var resp = http.get(url) catch return .fetch_failed; defer resp.deinit(); - if (resp.status != 200) return null; - if (resp.body.len == 0 or resp.body.len > max_formula_rb_bytes) return null; + if (resp.status != 200) return .fetch_failed; + if (resp.body.len == 0 or resp.body.len > max_formula_rb_bytes) return .fetch_failed; var actual_hex: [pins.SHA256_HEX_LEN]u8 = undefined; pins.sha256Hex(resp.body, &actual_hex); - if (!std.mem.eql(u8, actual_hex[0..], expected_hash)) return null; + if (!std.mem.eql(u8, actual_hex[0..], expected_hash)) return .fetch_failed; - return extractPostInstallFromSource(allocator, resp.body); + if (extractPostInstallFromSource(allocator, resp.body)) |body| { + return .{ .body = body }; + } + return .body_not_found; +} + +/// Fetch a formula's .rb source from the pinned homebrew-core commit, +/// verify its SHA256 against the embedded manifest, and extract the +/// post_install body. +/// +/// This path is the fallback when the homebrew-core tap is not cloned +/// locally. It refuses anything whose hash isn't pre-authorized in +/// `pins_manifest.txt` — a floating-HEAD fetch would give an attacker +/// who controls the raw.githubusercontent.com response (MITM with a +/// valid cert, compromised CDN edge, branch rewrite) a direct path to +/// code execution via `--use-system-ruby`. +/// +/// Returns the post_install body or null on any fetch / verify / parse +/// failure. All failures are silent-to-caller; the caller decides +/// whether to warn. +pub fn fetchPostInstallFromGitHub(io: std.Io, environ: std.process.Environ, allocator: std.mem.Allocator, name: []const u8) ?[]const u8 { + return switch (fetchPostInstallFromGitHubTagged(io, environ, allocator, name)) { + .body => |b| b, + .body_not_found, .fetch_failed => null, + }; } /// Extract post_install body + any sibling `def` blocks at the same indent, @@ -206,23 +237,65 @@ pub fn extractPostInstallFromSource(allocator: std.mem.Allocator, source: []cons return out.toOwnedSlice(allocator) catch return null; } +/// Which arm of the local-tap path was the last one to fail. Carried +/// from `resolvePostInstallBody` into the failure classifier so the +/// returned RubyError reflects the deepest-known reason rather than a +/// blanket TapNotFound. +const LocalArmFailure = enum { + /// `findHomebrewCoreTap` returned null — no clone on disk. + no_tap, + /// Tap clone exists but no `.rb` for this formula in either layout. + no_rb, + /// Tap + `.rb` exist but the body extractor returned null. + body_not_extracted, +}; + +/// Pure decision: pick the most actionable RubyError given the local +/// arm's failure mode and the fetch arm's outcome. Operators triage by +/// the variant, so we surface the deepest-known reason — e.g. a hash +/// mismatch becomes `FetchFailed`, not `TapNotFound`. +fn classifyResolveFailure(local: LocalArmFailure, fetch_failure: FetchOutcome) RubyError { + return switch (fetch_failure) { + .body => unreachable, // success isn't a failure to classify + .body_not_found => RubyError.PostInstallBodyNotFound, + .fetch_failed => switch (local) { + .no_tap => RubyError.TapNotFound, + .no_rb => RubyError.FetchFailed, + .body_not_extracted => RubyError.PostInstallBodyNotFound, + }, + }; +} + /// Resolve a formula's post_install body. Tries the on-disk /// homebrew-core tap first; on miss, falls back to the hash-pinned /// GitHub fetch so API-only Homebrew installs (no tap clone) still -/// reach a usable body. Caller owns the returned slice when non-null. +/// reach a usable body. Caller owns the returned slice on success. +/// +/// Failure surfaces a distinguishing RubyError — TapNotFound, +/// FetchFailed, or PostInstallBodyNotFound — so operators triaging an +/// install crash see the actual cause rather than a single catch-all. pub fn resolvePostInstallBody( io: std.Io, environ: std.process.Environ, allocator: std.mem.Allocator, name: []const u8, -) ?[]const u8 { +) RubyError![]const u8 { + var local: LocalArmFailure = .no_tap; if (findHomebrewCoreTap(io)) |tap_path| { var rb_buf: [1024]u8 = undefined; if (resolveFormulaRbPath(io, &rb_buf, tap_path, name)) |rb_path| { if (extractPostInstallBody(io, allocator, rb_path)) |body| return body; + local = .body_not_extracted; + } else { + local = .no_rb; } } - return fetchPostInstallFromGitHub(io, environ, allocator, name); + + const fetch = fetchPostInstallFromGitHubTagged(io, environ, allocator, name); + return switch (fetch) { + .body => |b| b, + else => classifyResolveFailure(local, fetch), + }; } /// Run the DSL parser over an isolated sibling-def block and report whether @@ -522,8 +595,9 @@ pub fn runPostInstall( // 2-4. Resolve the post_install body. Local homebrew-core tap is // preferred; hash-pinned GitHub fetch is the fallback so // API-only Homebrew installs (no tap clone) still resolve. - const body = resolvePostInstallBody(io, environ, allocator, name) orelse - return RubyError.TapNotFound; + // The resolver returns a distinguishing tag so a hash mismatch + // doesn't get filed under "no local tap". + const body = try resolvePostInstallBody(io, environ, allocator, name); defer allocator.free(body); return runPostInstallWithBody(io, environ, allocator, name, version, prefix, body, stdio); } @@ -632,3 +706,31 @@ pub fn runPostInstallWithBody( ) catch return RubyError.PostInstallFailed; if (exit_code != 0) return RubyError.PostInstallFailed; } + +test "classifyResolveFailure: fetch returned source but no body folds onto PostInstallBodyNotFound" { + // body_not_found is fetch-arm-only — local arm doesn't matter. + const cases = [_]LocalArmFailure{ .no_tap, .no_rb, .body_not_extracted }; + for (cases) |local| { + try std.testing.expectEqual( + RubyError.PostInstallBodyNotFound, + classifyResolveFailure(local, .body_not_found), + ); + } +} + +test "classifyResolveFailure: fetch_failed maps to the deepest local-arm reason" { + // The whole point of the split: hash mismatch under a populated tap + // surfaces FetchFailed, not TapNotFound. + try std.testing.expectEqual( + RubyError.TapNotFound, + classifyResolveFailure(.no_tap, .fetch_failed), + ); + try std.testing.expectEqual( + RubyError.FetchFailed, + classifyResolveFailure(.no_rb, .fetch_failed), + ); + try std.testing.expectEqual( + RubyError.PostInstallBodyNotFound, + classifyResolveFailure(.body_not_extracted, .fetch_failed), + ); +} diff --git a/tests/ruby_subprocess_test.zig b/tests/ruby_subprocess_test.zig index 069af73..9e95944 100644 --- a/tests/ruby_subprocess_test.zig +++ b/tests/ruby_subprocess_test.zig @@ -498,6 +498,62 @@ test "describeError covers every RubyError variant with a user hint" { } } +test "describeError distinguishes the four post_install source-resolution failure modes" { + // Operators triage by the variant. Pre-fix all four cases collapsed + // onto TapNotFound; the tags must now read distinctly so a hash + // mismatch isn't filed under "no local tap". + const tap = ruby.describeError(ruby.RubyError.TapNotFound); + const formula_src = ruby.describeError(ruby.RubyError.FormulaSourceNotFound); + const fetch = ruby.describeError(ruby.RubyError.FetchFailed); + const body = ruby.describeError(ruby.RubyError.PostInstallBodyNotFound); + + try testing.expect(!std.mem.eql(u8, tap, formula_src)); + try testing.expect(!std.mem.eql(u8, tap, fetch)); + try testing.expect(!std.mem.eql(u8, tap, body)); + try testing.expect(!std.mem.eql(u8, formula_src, fetch)); + try testing.expect(!std.mem.eql(u8, formula_src, body)); + try testing.expect(!std.mem.eql(u8, fetch, body)); +} + +test "resolvePostInstallBody surfaces a distinguishing tag instead of collapsing onto TapNotFound" { + // CI hosts have no homebrew-core clone; an obviously-bogus name has + // no pinned manifest entry, so the fetch refuses fail-closed without + // a network round-trip. On a brew-equipped dev box the local tap is + // present but won't carry this name, so the resolver routes to + // FetchFailed instead. Pre-fix both paths returned TapNotFound. + var threaded: std.Io.Threaded = .init(testing.allocator, .{ .environ = testEnviron() }); + defer threaded.deinit(); + const result = ruby.resolvePostInstallBody( + threaded.io(), + testEnviron(), + testing.allocator, + "__malt_d10_unknown_formula__", + ); + if (result) |_| { + return error.TestUnexpectedResult; + } else |err| { + const expected = if (ruby.findHomebrewCoreTap(testIo()) == null) + ruby.RubyError.TapNotFound + else + ruby.RubyError.FetchFailed; + try testing.expectEqual(expected, err); + } +} + +test "fetchPostInstallFromGitHub keeps its ?[]const u8 contract for CLI callers" { + // doctor + install rely on the optional-slice signature; the tagged + // variant is internal. An unknown name short-circuits before any + // network I/O via the manifest miss. + var threaded: std.Io.Threaded = .init(testing.allocator, .{ .environ = testEnviron() }); + defer threaded.deinit(); + try testing.expect(ruby.fetchPostInstallFromGitHub( + threaded.io(), + testEnviron(), + testing.allocator, + "__malt_d10_unknown_formula__", + ) == null); +} + test "detectRuby returns a heap-owned slice that the caller can free" { // On any machine that has Ruby available, the contract requires the // returned slice to be allocator-owned so the call site can pair it