Skip to content

fix(runtime): secure file writes through symlinked parents #411

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
phall1 wants to merge 3 commits into
base: main
Choose a base branch
from
+255 −6
Open

fix(runtime): secure file writes through symlinked parents #411

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea2f8d4
fix(runtime): allow symlinked file parents
Aug 18, 2026
08ed921
fix(runtime): secure file write destinations
Aug 18, 2026
8486b2c
fix(runtime): inspect opened file handles
Aug 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 61 additions & 6 deletions src/runtime/effects.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7044,11 +7044,14 @@ pub fn Effects(comptime Msg: type) type {
/// `max_effects` slots and the key space with spawns and fetches.
pub fn writeFile(self: *Self, options: WriteFileOptions) void {
if (options.path.len == 0 or options.path.len > max_effect_file_path_bytes or
options.bytes.len > max_effect_file_bytes)
options.bytes.len > max_effect_file_bytes or pathEndsWithSeparator(options.path))
{
return self.rejectFile(options.key, .write, options.on_result);
}
var access = self.resolvedFileAccess(options.path, .{ .create_parents = true }) orelse return self.rejectFileExternal(options.key, .write, options.on_result);
var access = self.resolvedFileAccess(options.path, .{
.create_parents = true,
.preserve_final_component = true,
}) orelse return self.rejectFileExternal(options.key, .write, options.on_result);
self.startFile(options.key, .write, &access, options.bytes, options.on_result);
}

Expand Down Expand Up @@ -16413,17 +16416,69 @@ pub fn Effects(comptime Msg: type) type {
ctx.done.store(true, .release);
}

const OpenedFileParent = struct {
dir: std.Io.Dir,
basename: []const u8,
};

fn openOrCreateFileParent(io: std.Io, file_path: []const u8) !OpenedFileParent {
const parent_path = std.fs.path.dirname(file_path) orelse ".";
const parent = std.Io.Dir.cwd().openDir(io, parent_path, .{ .follow_symlinks = true }) catch |err| switch (err) {
error.FileNotFound => create: {
try std.Io.Dir.cwd().createDirPath(io, parent_path);
// A newly created final component must still be a directory,
// not a symlink substituted between creation and opening.
break :create try std.Io.Dir.cwd().openDir(io, parent_path, .{ .follow_symlinks = false });
},
else => return err,
};
return .{ .dir = parent, .basename = std.fs.path.basename(file_path) };
}

fn pathEndsWithSeparator(path: []const u8) bool {
const last = path[path.len - 1];
return last == '/' or (builtin.os.tag == .windows and last == '\\');
}

fn writeThroughOpenedParent(dir: std.Io.Dir, io: std.Io, basename: []const u8, bytes: []const u8) !void {
const existing = dir.openFile(io, basename, .{
.mode = .write_only,
.allow_directory = false,
.follow_symlinks = false,
}) catch |err| switch (err) {
error.FileNotFound, error.SymLinkLoop => null,
else => return err,
};
if (existing) |file| {
const stat = file.stat(io) catch |err| {
file.close(io);
return err;
};
if (stat.kind != .file and stat.kind != .sym_link) {
defer file.close(io);
try file.writeStreamingAll(io, bytes);
return;
}
file.close(io);
}

var atomic = try dir.createFileAtomic(io, basename, .{ .replace = true });
defer atomic.deinit(io);
try atomic.file.writePositionalAll(io, bytes, 0);
try atomic.file.sync(io);
try atomic.replace(io);
}

fn runFileOp(ctx: *FileWorkerContext, io: std.Io) EffectFileOutcome {
if (ctx.missing) return if (ctx.op == .stat) .ok else .not_found;
const dir = ctx.parent orelse std.Io.Dir.cwd();
const file_path = if (ctx.parent != null) ctx.basename() else ctx.path();
switch (ctx.op) {
.write => {
if (ctx.parent == null) {
if (std.fs.path.dirname(file_path)) |parent| {
dir.createDirPath(io, parent) catch |err| return fileOpFailure(err);
}
dir.writeFile(io, .{ .sub_path = file_path, .data = ctx.payload() }) catch |err| return fileOpFailure(err);
var opened_parent = openOrCreateFileParent(io, file_path) catch |err| return fileOpFailure(err);
defer opened_parent.dir.close(io);
writeThroughOpenedParent(opened_parent.dir, io, opened_parent.basename, ctx.payload()) catch |err| return fileOpFailure(err);
return .ok;
}
var atomic = dir.createFileAtomic(io, file_path, .{ .make_path = ctx.parent == null, .replace = true }) catch |err| return fileOpFailure(err);
Expand Down
194 changes: 194 additions & 0 deletions src/runtime/effects_file_tests.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,142 @@ test "real executor writes a file (creating parent dirs) and reads it back" {
}
}

test "real executor writes through an existing symlinked parent" {
if (builtin.os.tag == .windows) return error.SkipZigTest;
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.createDir(io, "target", .default_dir);
try tmp.dir.symLink(io, "target", "alias", .{ .is_directory = true });

var h = try Harness.create();
defer h.destroy();
var path_buffer: [256]u8 = undefined;
test_path = try std.fmt.bufPrint(&path_buffer, ".zig-cache/tmp/{s}/alias/session.json", .{tmp.sub_path[0..]});
test_bytes = "through parent link";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);

try std.testing.expectEqual(effects_mod.EffectFileOutcome.ok, h.app_state.model.last_outcome.?);
const on_disk = try tmp.dir.readFileAlloc(io, "target/session.json", std.testing.allocator, .limited(64));
defer std.testing.allocator.free(on_disk);
try std.testing.expectEqualStrings("through parent link", on_disk);
}

test "real executor creates missing descendants below a symlinked parent" {
if (builtin.os.tag == .windows) return error.SkipZigTest;
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.createDir(io, "target", .default_dir);
try tmp.dir.symLink(io, "target", "alias", .{ .is_directory = true });

var h = try Harness.create();
defer h.destroy();
var path_buffer: [256]u8 = undefined;
test_path = try std.fmt.bufPrint(&path_buffer, ".zig-cache/tmp/{s}/alias/missing/nested/session.json", .{tmp.sub_path[0..]});
test_bytes = "nested through parent link";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);

try std.testing.expectEqual(effects_mod.EffectFileOutcome.ok, h.app_state.model.last_outcome.?);
const on_disk = try tmp.dir.readFileAlloc(io, "target/missing/nested/session.json", std.testing.allocator, .limited(64));
defer std.testing.allocator.free(on_disk);
try std.testing.expectEqualStrings("nested through parent link", on_disk);
}

test "real executor writes an absolute path" {
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
var absolute_storage: [std.Io.Dir.max_path_bytes]u8 = undefined;
const absolute_len = try tmp.dir.realPath(io, &absolute_storage);

var h = try Harness.create();
defer h.destroy();
var path_buffer: [std.Io.Dir.max_path_bytes]u8 = undefined;
test_path = try std.fmt.bufPrint(&path_buffer, "{s}/absolute.json", .{absolute_storage[0..absolute_len]});
test_bytes = "absolute";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);

try std.testing.expectEqual(effects_mod.EffectFileOutcome.ok, h.app_state.model.last_outcome.?);
const on_disk = try tmp.dir.readFileAlloc(io, "absolute.json", std.testing.allocator, .limited(64));
defer std.testing.allocator.free(on_disk);
try std.testing.expectEqualStrings("absolute", on_disk);
}

test "real executor rejects write paths with trailing separators" {
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
var h = try Harness.create();
defer h.destroy();

var path_buffer: [256]u8 = undefined;
test_path = try std.fmt.bufPrint(&path_buffer, ".zig-cache/tmp/{s}/destination/", .{tmp.sub_path[0..]});
test_bytes = "not written";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);

try std.testing.expectEqual(effects_mod.EffectFileOutcome.rejected, h.app_state.model.last_outcome.?);
}

test "real executor replaces a final symlink without overwriting its target" {
if (builtin.os.tag == .windows) return error.SkipZigTest;
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.writeFile(io, .{ .sub_path = "target.bin", .data = "keep me" });
try tmp.dir.symLink(io, "target.bin", "alias.bin", .{});

var h = try Harness.create();
defer h.destroy();
var path_buffer: [256]u8 = undefined;
test_path = try std.fmt.bufPrint(&path_buffer, ".zig-cache/tmp/{s}/alias.bin", .{tmp.sub_path[0..]});
test_bytes = "replacement";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);

try std.testing.expectEqual(effects_mod.EffectFileOutcome.ok, h.app_state.model.last_outcome.?);
const target = try tmp.dir.readFileAlloc(io, "target.bin", std.testing.allocator, .limited(64));
defer std.testing.allocator.free(target);
try std.testing.expectEqualStrings("keep me", target);
const replacement_stat = try tmp.dir.statFile(io, "alias.bin", .{ .follow_symlinks = false });
try std.testing.expectEqual(std.Io.File.Kind.file, replacement_stat.kind);
}

test "bound real executor replaces a final symlink without overwriting its target" {
if (builtin.os.tag == .windows) return error.SkipZigTest;
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
try tmp.dir.createDir(io, "app-data", .default_dir);
try tmp.dir.writeFile(io, .{ .sub_path = "app-data/target.bin", .data = "keep me" });
try tmp.dir.symLink(io, "target.bin", "app-data/alias.bin", .{});

var root_buffer: [256]u8 = undefined;
const root = try std.fmt.bufPrint(&root_buffer, ".zig-cache/tmp/{s}/app-data", .{tmp.sub_path[0..]});
var path_buffer: [256]u8 = undefined;
const alias = try std.fmt.bufPrint(&path_buffer, "{s}/alias.bin", .{root});
var h = try Harness.create();
defer h.destroy();
h.app_state.effects.bindFileAccess(.{ .roots = &.{root}, .permitted = false, .enforce = true });
test_path = alias;
test_bytes = "replacement";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);

try std.testing.expectEqual(effects_mod.EffectFileOutcome.ok, h.app_state.model.last_outcome.?);
const target = try tmp.dir.readFileAlloc(io, "app-data/target.bin", std.testing.allocator, .limited(64));
defer std.testing.allocator.free(target);
try std.testing.expectEqualStrings("keep me", target);
const replacement = try tmp.dir.readFileAlloc(io, "app-data/alias.bin", std.testing.allocator, .limited(64));
defer std.testing.allocator.free(replacement);
try std.testing.expectEqualStrings("replacement", replacement);
const replacement_stat = try tmp.dir.statFile(io, "app-data/alias.bin", .{ .follow_symlinks = false });
try std.testing.expectEqual(std.Io.File.Kind.file, replacement_stat.kind);
}

test "real executor reports missing files as not_found" {
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
Expand Down Expand Up @@ -1041,6 +1177,64 @@ fn makeFifo(path: []const u8) !void {
try std.testing.expect(result.term == .exited and result.term.exited == 0);
}

const FifoRead = struct {
path: []const u8,
bytes: [64]u8 = undefined,
len: usize = 0,
err: ?anyerror = null,

fn run(self: *FifoRead) void {
const io = std.testing.io;
var file = std.Io.Dir.cwd().openFile(io, self.path, .{}) catch |err| {
self.err = err;
return;
};
defer file.close(io);
while (self.len < self.bytes.len) {
var slices: [1][]u8 = .{self.bytes[self.len..]};
const len = file.readStreaming(io, &slices) catch |err| switch (err) {
error.EndOfStream => return,
else => {
self.err = err;
return;
},
};
if (len == 0) return;
self.len += len;
}
}
};

test "real executor streams to the exact no-follow-opened FIFO handle" {
if (builtin.os.tag == .windows) return error.SkipZigTest;
const io = std.testing.io;
var tmp = std.testing.tmpDir(.{});
defer tmp.cleanup();
var fifo_path_buffer: [256]u8 = undefined;
const fifo_path = try std.fmt.bufPrint(&fifo_path_buffer, ".zig-cache/tmp/{s}/delivery.fifo", .{tmp.sub_path[0..]});
try makeFifo(fifo_path);

var read: FifoRead = .{ .path = fifo_path };
const reader = try std.Thread.spawn(.{}, FifoRead.run, .{&read});
var reader_joined = false;
defer if (!reader_joined) reader.join();

var h = try Harness.create();
defer h.destroy();
test_path = fifo_path;
test_bytes = "fifo payload";
try h.app_state.dispatch(&h.harness.runtime, 1, .save);
try waitForRealResult(&h, 1);
reader.join();
reader_joined = true;

try std.testing.expect(read.err == null);
try std.testing.expectEqual(effects_mod.EffectFileOutcome.ok, h.app_state.model.last_outcome.?);
try std.testing.expectEqualStrings("fifo payload", read.bytes[0..read.len]);
const stat = try tmp.dir.statFile(io, "delivery.fifo", .{ .follow_symlinks = false });
try std.testing.expectEqual(std.Io.File.Kind.named_pipe, stat.kind);
}

test "teardown abandons a file worker stuck on a reader-less FIFO and leaks its world loudly" {
if (builtin.os.tag == .windows) return error.SkipZigTest;
const io = std.testing.io;
Expand Down