a2c2b67bd83a

diff guards against a u32 underflow panic by returning a friendly message when no generation is marked current or too few generations exist, instead of falling into current - 1 arithmetic (NINA-007)

time
2026-07-02 13:38 UTC · 3 months ago
machine
koh2
agent
claude-sonnet-5
session
fable audit fix — commands.zig
parent
d5b8315c6ade
2 files src/commands.zig+72 −11src/output.zig+7 −0
src/commands.zig+72 −11
@@ -273,24 +273,85 @@273273     output.statusRow(machine.name, gen, st.age, st.size, true);274274 }275275 276-pub fn diff(ctx: Ctx) !void {277-    const gens = try exec.getGenerations(ctx.gpa, ctx.io, &ctx.machine);278-    defer ctx.gpa.free(gens);276+// Pure decision for `nina diff`'s generation targets, split out so it can be277+// unit tested without shelling out. Returns null when there's nothing safe to278+// diff — no generation is marked current (a non-standard profile layout, a279+// transient readlink failure, or a non-GNU remote host can all produce this)280+// or fewer than two generations exist and no explicit pair was given. Nina281+// ships ReleaseSafe, so leaving `current - 1` reachable with current == 0282+// would be a u32 underflow panic, not silent wraparound.283+fn diffTargets(gens: []const types.GenerationInfo, args: []const []const u8) ?struct { a: u32, b: u32 } {279284     var current: u32 = 0;280285     var prev: u32 = 0;281286     for (gens) |g| {282287         if (g.current) current = g.number;283288         if (!g.current and g.number > prev and g.number < current) prev = g.number;284289     }285-    if (ctx.args.len >= 2) {286-        const a = std.fmt.parseInt(u32, ctx.args[0], 10) catch current - 1;287-        const b = std.fmt.parseInt(u32, ctx.args[1], 10) catch current;288-        output.diffHeader(a, b, ctx.machine.name);289-        if (try exec.diffClosures(ctx.io, &ctx.machine, ctx.gpa, a, b) != 0) return error.ProcessFailed;290-    } else {291-        output.diffHeader(if (prev > 0) prev else current - 1, current, ctx.machine.name);292-        if (try exec.diffClosures(ctx.io, &ctx.machine, ctx.gpa, if (prev > 0) prev else current - 1, current) != 0) return error.ProcessFailed;290+    if (args.len >= 2) {291+        const a = std.fmt.parseInt(u32, args[0], 10) catch blk: {292+            if (current == 0) return null;293+            break :blk current - 1;294+        };295+        const b = std.fmt.parseInt(u32, args[1], 10) catch blk: {296+            if (current == 0) return null;297+            break :blk current;298+        };299+        return .{ .a = a, .b = b };293300     }301+    if (current == 0 or gens.len < 2) return null;302+    return .{ .a = if (prev > 0) prev else current - 1, .b = current };303+}304+305+pub fn diff(ctx: Ctx) !void {306+    const gens = try exec.getGenerations(ctx.gpa, ctx.io, &ctx.machine);307+    defer ctx.gpa.free(gens);308+    const targets = diffTargets(gens, ctx.args) orelse {309+        output.diffCannotDetermine();310+        return;311+    };312+    output.diffHeader(targets.a, targets.b, ctx.machine.name);313+    if (try exec.diffClosures(ctx.io, &ctx.machine, ctx.gpa, targets.a, targets.b) != 0) return error.ProcessFailed;314+}315+316+test "diffTargets returns null when no generation is marked current" {317+    const gens = [_]types.GenerationInfo{318+        .{ .number = 1, .date = "", .time = "", .current = false },319+        .{ .number = 2, .date = "", .time = "", .current = false },320+    };321+    try std.testing.expect(diffTargets(&gens, &.{}) == null);322+}323+324+test "diffTargets returns null with fewer than two generations and no explicit args" {325+    const gens = [_]types.GenerationInfo{326+        .{ .number = 1, .date = "", .time = "", .current = true },327+    };328+    try std.testing.expect(diffTargets(&gens, &.{}) == null);329+}330+331+test "diffTargets falls back to prev/current when both are known" {332+    const gens = [_]types.GenerationInfo{333+        .{ .number = 1, .date = "", .time = "", .current = false },334+        .{ .number = 2, .date = "", .time = "", .current = true },335+    };336+    const targets = diffTargets(&gens, &.{}).?;337+    try std.testing.expectEqual(@as(u32, 1), targets.a);338+    try std.testing.expectEqual(@as(u32, 2), targets.b);339+}340+341+test "diffTargets returns null when explicit args fail to parse and no current generation exists" {342+    const gens = [_]types.GenerationInfo{343+        .{ .number = 1, .date = "", .time = "", .current = false },344+    };345+    try std.testing.expect(diffTargets(&gens, &.{ "not-a-number", "2" }) == null);346+}347+348+test "diffTargets uses explicit args verbatim when both parse" {349+    const gens = [_]types.GenerationInfo{350+        .{ .number = 1, .date = "", .time = "", .current = false },351+    };352+    const targets = diffTargets(&gens, &.{ "3", "5" }).?;353+    try std.testing.expectEqual(@as(u32, 3), targets.a);354+    try std.testing.expectEqual(@as(u32, 5), targets.b);294355 }295356 296357 pub fn mood(ctx: Ctx) !void {

view file

src/output.zig+7 −0
@@ -584,6 +584,13 @@584584     p("{s}gen {d} -> gen {d}  {s}\n\n", .{ hdr(), a, b, machine });585585 }586586 587+// Reached when no generation is marked current (non-standard profile layout,588+// a transient readlink failure, or a non-GNU remote host) or there are too589+// few generations to infer a pair — nothing safe to diff against.590+pub fn diffCannotDetermine() void {591+    p("{s}cannot determine current generation — nothing to diff\n", .{if (color_enabled) WARN_C else WARN_P});592+}593+587594 pub fn diffRow(entry: types.DiffEntry) void {588595     switch (entry.op) {589596         .add => p("   {s}+{s} {s}   {s}\n", .{ c(GREEN), c(RESET), entry.package, entry.new_version orelse "" }),

view file