fix(dsl): confine native read sources

src/core/dsl/builtins/inreplace.zig

@@ -38,8 +38,14 @@ pub fn inreplace(ctx: ExecCtx, _: ?Value, args: []const Value) BuiltinError!Valu
     sandbox.validateWriteDir(ctx.io, path, ctx.cellar_path, ctx.malt_prefix) catch
         return BuiltinError.PathSandboxViolation;
 
-    // Read file contents
-    const content = read.readFileAllAbsolute(ctx.io, ctx.allocator, path, 4 * 1024 * 1024) catch {
+    // Open the source itself without following a final symlink. The descriptor
+    // keeps the read bound to the object that passed the confinement check.
+    const file = sandbox.openSourceNoFollow(ctx.io, path, ctx.cellar_path, ctx.malt_prefix) catch |e| switch (e) {
+        error.PathSandboxViolation => return BuiltinError.PathSandboxViolation,
+        else => return Value{ .nil = {} },
+    };
+    defer file.close(ctx.io);
+    const content = read.readFileAll(ctx.io, ctx.allocator, file, 4 * 1024 * 1024) catch {
         return Value{ .nil = {} };
     };
 
@@ -146,3 +152,40 @@ test "atomic-write failure message names the failing step" {
     try std.testing.expect(std.mem.indexOf(u8, rename_msg, "RenameFailed") != null);
     try std.testing.expect(std.mem.indexOf(u8, rename_msg, "CrossDeviceLink") != null);
 }
+
+test "inreplace refuses to read through a source symlink outside the keg" {
+    const io = std.Options.debug_io;
+    var arena = std.heap.ArenaAllocator.init(std.testing.allocator);
+    defer arena.deinit();
+    const alloc = arena.allocator();
+    const base = try std.fmt.allocPrint(alloc, "/tmp/malt_inreplace_source_{d}", .{std.c.getpid()});
+    std.Io.Dir.cwd().deleteTree(io, base) catch {};
+    defer std.Io.Dir.cwd().deleteTree(io, base) catch {};
+
+    const keg = try std.fmt.allocPrint(alloc, "{s}/keg", .{base});
+    const victim = try std.fmt.allocPrint(alloc, "{s}/victim", .{base});
+    const link = try std.fmt.allocPrint(alloc, "{s}/config", .{keg});
+    try std.Io.Dir.cwd().createDirPath(io, keg);
+    {
+        const f = try std.Io.Dir.createFileAbsolute(io, victim, .{});
+        defer f.close(io);
+        try f.writeStreamingAll(io, "PRIVATE");
+    }
+    try std.Io.Dir.symLinkAbsolute(io, victim, link, .{});
+
+    const ctx: ExecCtx = .{
+        .allocator = alloc,
+        .io = io,
+        .environ = .empty,
+        .cellar_path = keg,
+        .malt_prefix = keg,
+    };
+    try std.testing.expectError(
+        BuiltinError.PathSandboxViolation,
+        inreplace(ctx, null, &.{
+            Value{ .string = link },
+            Value{ .string = "PRIVATE" },
+            Value{ .string = "PRIVATE" },
+        }),
+    );
+}

src/core/dsl/builtins/pathname.zig

@@ -102,7 +102,12 @@ pub fn write(ctx: ExecCtx, receiver: ?Value, args: []const Value) BuiltinError!V
 pub fn read(ctx: ExecCtx, receiver: ?Value, _: []const Value) BuiltinError!Value {
     const path = try receiverPath(ctx.allocator, receiver);
     if (path.len == 0) return Value{ .string = "" };
-    const content = fs_read.readFileAllAbsolute(ctx.io, ctx.allocator, path, 1024 * 1024) catch {
+    const file = sandbox.openSourceNoFollow(ctx.io, path, ctx.cellar_path, ctx.malt_prefix) catch |e| switch (e) {
+        error.PathSandboxViolation => return BuiltinError.PathSandboxViolation,
+        else => return Value{ .string = "" },
+    };
+    defer file.close(ctx.io);
+    const content = fs_read.readFileAll(ctx.io, ctx.allocator, file, 1024 * 1024) catch {
         return Value{ .string = "" };
     };
     return Value{ .string = content };

src/core/dsl/sandbox.zig

@@ -227,6 +227,18 @@ pub fn openTargetNoFollow(
     return .{ .handle = fd, .flags = .{ .nonblocking = false } };
 }
 
+/// Open a confined source without following its final component. Keeping the
+/// returned descriptor open binds subsequent reads to the checked object and
+/// closes the leaf-symlink and check-then-open windows.
+pub fn openSourceNoFollow(
+    io: std.Io,
+    source_path: []const u8,
+    cellar_path: []const u8,
+    malt_prefix: []const u8,
+) (SandboxError || std.posix.OpenError)!std.Io.File {
+    return openTargetNoFollow(io, source_path, cellar_path, malt_prefix, .{ .write = false });
+}
+
 const fs_test_io = std.Options.debug_io;
 
 var scratch_seq: std.atomic.Value(u32) = .init(0);

src/core/post_install_steps.zig

@@ -15,6 +15,7 @@ const sandbox_macos = @import("sandbox/macos.zig");
 const fallback_log = @import("dsl/fallback_log.zig");
 const atomic = @import("../fs/atomic.zig");
 const clonefile = @import("../fs/clonefile.zig");
+const fs_read = @import("../fs/read.zig");
 const text_replace = @import("../text_replace.zig");
 
 pub const FallbackLog = fallback_log.FallbackLog;
@@ -732,25 +733,14 @@ fn stepInreplace(ctx: StepsCtx, obj: std.json.ObjectMap) bool {
         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);
+    const file = sandbox.openSourceNoFollow(ctx.io, path, ctx.keg_path, ctx.prefix) catch |e| {
+        if (e == error.PathSandboxViolation) logViolation(ctx, path);
         return false;
     };
-    file.close(ctx.io);
-    if (read < content.len) return false;
+    defer file.close(ctx.io);
+    const stat = file.stat(ctx.io) catch return false;
+    if (stat.size > 4 * 1024 * 1024) return false;
+    const content = fs_read.readFileAll(ctx.io, ctx.allocator, file, 4 * 1024 * 1024) catch return false;
 
     const updated = if (getFlag(obj, "regexp")) blk: {
         const literal = anchoredLineLiteral(before) orelse {
@@ -1482,6 +1472,36 @@ test "execute substitutes a literal inreplace and resolves the invoking user" {
     try testing.expectEqualStrings("username: \"ada\"\n", try readBack(&h, path));
 }
 
+test "execute refuses inreplace through a source symlink outside the prefix" {
+    var h = try TestHarness.init();
+    defer h.deinit();
+    const a = h.arena.allocator();
+
+    var outside = try Scratch.init("steps_inreplace_source");
+    defer outside.deinit();
+    try std.Io.Dir.cwd().createDirPath(h.io, outside.base);
+    const victim = outside.p("/victim");
+    {
+        const f = try std.Io.Dir.createFileAbsolute(h.io, victim, .{});
+        defer f.close(h.io);
+        try f.writeStreamingAll(h.io, "PRIVATE");
+    }
+
+    const etc = try std.fmt.allocPrint(a, "{s}/etc", .{h.prefix});
+    try std.Io.Dir.cwd().createDirPath(h.io, etc);
+    const link = try std.fmt.allocPrint(a, "{s}/leak.conf", .{etc});
+    try std.Io.Dir.symLinkAbsolute(h.io, victim, link, .{});
+
+    _ = execute(h.ctx(), try testFormulaJson(&h,
+        \\[{"type":"inreplace","path":{"base":"etc","path":"leak.conf"},
+        \\  "before":"PRIVATE","after":"PRIVATE"}]
+    ));
+
+    try testing.expect(h.flog.hasFatal());
+    var target_buf: [std.fs.max_path_bytes]u8 = undefined;
+    _ = try std.Io.Dir.readLinkAbsolute(h.io, link, &target_buf);
+}
+
 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.

src/fs/read.zig

@@ -13,6 +13,13 @@ const std = @import("std");
 pub fn readFileAllAbsolute(io: std.Io, allocator: std.mem.Allocator, abs_path: []const u8, max_bytes: usize) ![]u8 {
     const f = try std.Io.Dir.openFileAbsolute(io, abs_path, .{});
     defer f.close(io);
+    return readFileAll(io, allocator, f, max_bytes);
+}
+
+/// Read from an already-open file descriptor. Security-sensitive callers use
+/// this after opening with `O_NOFOLLOW`, keeping the read bound to the object
+/// that passed their confinement check.
+pub fn readFileAll(io: std.Io, allocator: std.mem.Allocator, f: std.Io.File, max_bytes: usize) ![]u8 {
     const st = try f.stat(io);
     const size = @min(@as(u64, max_bytes), st.size);
     const buf = try allocator.alloc(u8, @intCast(size));

tests/dsl_builtins_test.zig

@@ -154,10 +154,65 @@ test "Pathname.read returns empty string for a missing file" {
     defer testing.allocator.free(root);
     defer test_io.deleteTreeAbsolute(std.Options.debug_io, root) catch {};
     const ctx = mkCtx(root);
-    const out = try pathname.read(ctx, Value{ .pathname = "/tmp/malt_dsl_read_missing_xyz" }, &.{});
+    const missing = try std.fmt.allocPrint(testing.allocator, "{s}/missing", .{root});
+    defer testing.allocator.free(missing);
+    const out = try pathname.read(ctx, Value{ .pathname = missing }, &.{});
     try testing.expectEqualStrings("", out.string);
 }
 
+test "Pathname.read refuses a source outside the sandbox" {
+    const root = try uniqueSandbox("read_escape_root");
+    defer testing.allocator.free(root);
+    defer test_io.deleteTreeAbsolute(std.Options.debug_io, root) catch {};
+    const outside = try uniqueSandbox("read_escape_outside");
+    defer testing.allocator.free(outside);
+    defer test_io.deleteTreeAbsolute(std.Options.debug_io, outside) catch {};
+    const victim = try std.fmt.allocPrint(testing.allocator, "{s}/private", .{outside});
+    defer testing.allocator.free(victim);
+    {
+        const f = try test_io.createFileAbsolute(std.Options.debug_io, victim, .{});
+        defer f.close(std.Options.debug_io);
+        try f.writeStreamingAll(std.Options.debug_io, "PRIVATE");
+    }
+
+    var arena = std.heap.ArenaAllocator.init(testing.allocator);
+    defer arena.deinit();
+    var ctx = mkCtx(root);
+    ctx.allocator = arena.allocator();
+    try testing.expectError(
+        pathname.BuiltinError.PathSandboxViolation,
+        pathname.read(ctx, Value{ .pathname = victim }, &.{}),
+    );
+}
+
+test "Pathname.read refuses a final symlink outside the sandbox" {
+    const root = try uniqueSandbox("read_symlink_root");
+    defer testing.allocator.free(root);
+    defer test_io.deleteTreeAbsolute(std.Options.debug_io, root) catch {};
+    const outside = try uniqueSandbox("read_symlink_outside");
+    defer testing.allocator.free(outside);
+    defer test_io.deleteTreeAbsolute(std.Options.debug_io, outside) catch {};
+    const victim = try std.fmt.allocPrint(testing.allocator, "{s}/private", .{outside});
+    defer testing.allocator.free(victim);
+    const link = try std.fmt.allocPrint(testing.allocator, "{s}/config", .{root});
+    defer testing.allocator.free(link);
+    {
+        const f = try test_io.createFileAbsolute(std.Options.debug_io, victim, .{});
+        defer f.close(std.Options.debug_io);
+        try f.writeStreamingAll(std.Options.debug_io, "PRIVATE");
+    }
+    try test_io.symLinkAbsolute(std.Options.debug_io, victim, link, .{});
+
+    var arena = std.heap.ArenaAllocator.init(testing.allocator);
+    defer arena.deinit();
+    var ctx = mkCtx(root);
+    ctx.allocator = arena.allocator();
+    try testing.expectError(
+        pathname.BuiltinError.PathSandboxViolation,
+        pathname.read(ctx, Value{ .pathname = link }, &.{}),
+    );
+}
+
 test "Pathname.children returns array of children; empty for missing dir" {
     const root = try uniqueSandbox("children");
     defer testing.allocator.free(root);