d4dd5ae4eb6e

self-update download uses a random mktemp path instead of a predictable /tmp/nina-update.bin, and the caller deletes it after install (NINA-004)

time
2026-07-02 13:24 UTC · 3 months ago
machine
koh2
agent
claude-sonnet-5
session
fable audit fix — exec.zig
parent
08ace490d251
2 files src/commands.zig+3 −0src/exec.zig+53 −3
src/commands.zig+3 −0
@@ -519,6 +519,9 @@519519         return;520520     };521521     defer ctx.gpa.free(tmp_path);522+    // Clean up the downloaded temp file whether install succeeds or fails —523+    // it's only ever a scratch copy verified against the release checksum.524+    defer std.Io.Dir.deleteFileAbsolute(ctx.io, tmp_path) catch {};522525 523526     output.updateSelfVerifying();524527     exec.verifyAndInstall(ctx.gpa, ctx.io, tmp_path, expected_hex, install_path) catch |e| switch (e) {

view file

src/exec.zig+53 −3
@@ -2032,10 +2032,15 @@20322032     return body;20332033 }20342034 2035-// Download a URL to a temp file. Returns the temp path (caller must gpa.free).2036-// The file is written to /tmp/nina-update-<pid> and left for the caller to use.2035+// Download a URL to a temp file. Returns the temp path (caller must gpa.free2036+// the string AND delete the file once done with it — this only reserves the2037+// name and writes the content). mktemp's random suffix keeps the path from2038+// being guessable, unlike a fixed name that a shared /tmp lets any local user2039+// symlink out from under a self-update.20372040 pub fn downloadToTemp(gpa: std.mem.Allocator, io: std.Io, url: []const u8) ![]u8 {2038-    const tmp_path = try gpa.dupe(u8, "/tmp/nina-update.bin");2041+    const tmp_raw = try captureLocal(gpa, io, &.{ "mktemp", "/tmp/nina-update.XXXXXX" });2042+    defer gpa.free(tmp_raw);2043+    const tmp_path = try gpa.dupe(u8, std.mem.trim(u8, tmp_raw, " \t\r\n"));20392044     errdefer gpa.free(tmp_path);20402045 20412046     var child = std.process.spawn(io, .{@@ -2050,6 +2055,51 @@20502055         else => return error.NetworkError,20512056     }20522057     return tmp_path;2058+}2059+2060+test "downloadToTemp uses a distinct random path per call and can be cleaned up (NINA-004)" {2061+    const a = std.testing.allocator;2062+    const io = std.testing.io;2063+2064+    var tmp = std.testing.tmpDir(.{});2065+    defer tmp.cleanup();2066+2067+    const payload = "nina download fixture payload\n";2068+    {2069+        const f = try tmp.dir.createFile(io, "source.bin", .{});2070+        defer f.close(io);2071+        try f.writePositionalAll(io, payload, 0);2072+    }2073+2074+    // curl supports file:// URLs, so this exercises the real downloadToTemp2075+    // implementation (real mktemp + real curl spawn) without any network.2076+    var real_path_buf: [std.Io.Dir.max_path_bytes]u8 = undefined;2077+    const real_path_len = try tmp.dir.realPathFile(io, "source.bin", &real_path_buf);2078+    const url = try std.fmt.allocPrint(a, "file://{s}", .{real_path_buf[0..real_path_len]});2079+    defer a.free(url);2080+2081+    const path1 = try downloadToTemp(a, io, url);2082+    defer a.free(path1);2083+    defer std.Io.Dir.deleteFileAbsolute(io, path1) catch {};2084+2085+    const path2 = try downloadToTemp(a, io, url);2086+    defer a.free(path2);2087+    defer std.Io.Dir.deleteFileAbsolute(io, path2) catch {};2088+2089+    // mktemp's random suffix must differ per call — a fixed path is2090+    // symlink-clobberable and shared across concurrent self-updates.2091+    try std.testing.expect(!std.mem.eql(u8, path1, path2));2092+    try std.testing.expect(!std.mem.eql(u8, path1, "/tmp/nina-update.bin"));2093+2094+    try std.testing.expect(pathExists(io, path1));2095+    const downloaded = try std.Io.Dir.readFileAlloc(.cwd(), io, path1, a, .unlimited);2096+    defer a.free(downloaded);2097+    try std.testing.expectEqualStrings(payload, downloaded);2098+2099+    // The caller owns cleanup (see the update command's post-install defer) —2100+    // confirm the mktemp'd file is actually removable and gone afterward.2101+    try std.Io.Dir.deleteFileAbsolute(io, path1);2102+    try std.testing.expect(!pathExists(io, path1));20532103 }20542104 20552105 // Get the current executable path into buf. Returns slice of buf.

view file