From e76b09c391d66346f8029ddec127baf3cadcf961 Mon Sep 17 00:00:00 2001
From: Travis Cole <11240+kelp@users.noreply.github.com>
Date: Mon, 30 Mar 2026 09:19:14 -0700
Subject: [PATCH] Add failing tests for env and free IMPORTANT audit findings

---
 src/env.zig                  |  69 ++++++++++++++++++++++++
 src/free.zig                 | 102 +++++++++++++++++++++++++++++++++++
 tests/utilities/env_test.sh  |  35 ++++++++++++
 tests/utilities/free_test.sh |  41 ++++++++++++++
 4 files changed, 247 insertions(+)

diff --git a/src/env.zig b/src/env.zig
index e1884e5..cb37cbd 100644
--- a/src/env.zig
+++ b/src/env.zig
@@ -1042,3 +1042,72 @@ test "env runEnv: -S splits string into assignment" {
     // Should NOT print a stub warning
     try testing.expectEqual(@as(usize, 0), stderr_buffer.items.len);
 }
+
+// ========== AUDIT WAVE 4: env IMPORTANT findings ==========
+
+// IMPORTANT: -0 with utility argument should be rejected
+// macOS spec: "Both -0 and utility may not be specified together."
+// -0 is only valid when printing the environment (no utility given).
+// Currently: parseArgs allows -0 with a command. runEnv should reject
+// this combination before executing. We verify via parseArgs that
+// both null_delimiter and command are set, which the fix should prevent.
+test "audit: env -0 with utility should be detected" {
+    // Parse args: -0 echo hello — both null_delimiter and command are set
+    const options = try parseArgs(testing.allocator, &.{ "-0", "echo", "hello" });
+    defer options.deinit(testing.allocator);
+    // The bug: both null_delimiter and command are accepted simultaneously.
+    // After the fix, runEnv should reject this combination (exit 125).
+    // For now, verify the parser accepts both (proving the bug exists):
+    try testing.expect(options.null_delimiter);
+    try testing.expect(options.command.len > 0);
+    // Now verify runEnv rejects the combination with exit 125.
+    // (This avoids spawning a child process in tests.)
+    var stdout_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stdout_buffer.deinit(testing.allocator);
+    var stderr_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stderr_buffer.deinit(testing.allocator);
+
+    // Use -i so no inherited env, and a nonexistent command to avoid spawn.
+    // But the validation should reject BEFORE reaching exec.
+    const exit_code = runEnv(testing.allocator, &.{ "-i", "-0", "NONEXISTENT_CMD_AUDIT_TEST_12345" }, stdout_buffer.writer(testing.allocator), stderr_buffer.writer(testing.allocator));
+    // Per spec, should exit 125 rejecting the -0+utility combination.
+    // Currently returns 127 (command not found) because validation is missing.
+    try testing.expectEqual(@as(u8, 125), exit_code);
+}
+
+// IMPORTANT: Flags after NAME=VALUE pairs should not be parsed as options
+// macOS spec: "The above options are only recognized when they are
+// specified before any name=value options." Once a NAME=VALUE token
+// is seen, subsequent flag-like tokens should start the command.
+// Currently: `env FOO=bar -u FOO` parses -u as a flag and unsets FOO.
+// Expected: -u should be treated as the start of the command.
+test "audit: env flags after NAME=VALUE should not be parsed" {
+    // env FOO=bar -u FOO — after FOO=bar, -u should be treated as command
+    const options = try parseArgs(testing.allocator, &.{ "FOO=bar", "-u", "FOO" });
+    defer options.deinit(testing.allocator);
+    // After the fix: -u should be treated as command start, not a flag.
+    // command should be ["-u", "FOO"], unsets should be empty.
+    try testing.expectEqual(@as(usize, 0), options.unsets.len);
+    try testing.expectEqual(@as(usize, 2), options.command.len);
+    try testing.expectEqualStrings("-u", options.command[0]);
+    try testing.expectEqualStrings("FOO", options.command[1]);
+}
+
+// IMPORTANT: -P should restrict utility search path, not set child's PATH
+// macOS spec: "-P replaces the directory search path used to locate
+// the utility". It should NOT appear in the child's environment.
+// Currently: `env -i -P /usr/bin FOO=bar` puts PATH=/usr/bin in output.
+// Expected: Only FOO=bar should appear (PATH should not be in child env).
+test "audit: env -P should not set PATH in child environment" {
+    var stdout_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stdout_buffer.deinit(testing.allocator);
+    var stderr_buffer = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stderr_buffer.deinit(testing.allocator);
+
+    // env -i -P /custom/path FOO=bar — print env with -i, -P, and assignment
+    // -P should NOT inject PATH into the child's environment
+    const exit_code = runEnv(testing.allocator, &.{ "-i", "-P", "/custom/path", "FOO=bar" }, stdout_buffer.writer(testing.allocator), stderr_buffer.writer(testing.allocator));
+    try testing.expectEqual(@as(u8, 0), exit_code);
+    // Only FOO=bar should be printed; PATH should NOT appear
+    try testing.expectEqualStrings("FOO=bar\n", stdout_buffer.items);
+}
diff --git a/src/free.zig b/src/free.zig
index 69439de..d6fd383 100644
--- a/src/free.zig
+++ b/src/free.zig
@@ -873,3 +873,105 @@ test "getMemInfo returns valid data" {
     // Used + free should not exceed total (approximately)
     try testing.expect(info.used <= info.total);
 }
+
+// ========== AUDIT WAVE 4: free IMPORTANT findings ==========
+
+// IMPORTANT: -w wide mode always shows 0 for the buffers column
+// GNU free -w shows the actual kernel buffer allocation in the buffers
+// column. Our implementation hardcodes 0 for buffers regardless of
+// platform, because MemInfo.buff_cache merges buffers + cached and
+// the individual buffers value is lost.
+// Currently: printReport wide mode shows 0 for buffers even when
+// buff_cache is nonzero. Expected: nonzero buffers value.
+test "audit: free -w wide mode should show nonzero buffers" {
+    var stdout_buf = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stdout_buf.deinit(testing.allocator);
+
+    // Mock data with nonzero buff_cache — buffers should be part of it
+    const info = MemInfo{
+        .total = 16 * 1024 * 1024 * 1024,
+        .used = 8 * 1024 * 1024 * 1024,
+        .free = 4 * 1024 * 1024 * 1024,
+        .shared = 512 * 1024 * 1024,
+        .buff_cache = 4 * 1024 * 1024 * 1024, // 4 GiB combined
+        .available = 12 * 1024 * 1024 * 1024,
+        .swap_total = 0,
+        .swap_used = 0,
+        .swap_free = 0,
+    };
+
+    try printReport(stdout_buf.writer(testing.allocator), info, .kibi, false, false, true);
+
+    const output = stdout_buf.items;
+    // In wide mode, the Mem: row has 7 numeric columns:
+    // total, used, free, shared, buffers, cache, available
+    // Find the Mem: line
+    const mem_line_start = std.mem.indexOf(u8, output, "Mem:") orelse
+        return error.TestExpectedEqual;
+    const mem_line_end = std.mem.indexOfScalarPos(u8, output, mem_line_start, '\n') orelse output.len;
+    const mem_line = output[mem_line_start..mem_line_end];
+
+    // Parse numeric values from the Mem: line
+    // Skip "Mem:" label, then extract whitespace-separated numbers
+    var values: [7]u64 = undefined;
+    var val_count: usize = 0;
+    var iter = std.mem.tokenizeScalar(u8, mem_line, ' ');
+    _ = iter.next(); // skip "Mem:"
+    while (iter.next()) |token| {
+        if (val_count < 7) {
+            values[val_count] = std.fmt.parseInt(u64, token, 10) catch continue;
+            val_count += 1;
+        }
+    }
+
+    // We should have 7 values
+    try testing.expectEqual(@as(usize, 7), val_count);
+
+    // values[4] is the buffers column — should be nonzero when
+    // buff_cache is nonzero, since buffers is a component of it
+    // Currently fails because buffers is hardcoded to 0
+    try testing.expect(values[4] > 0);
+}
+
+// IMPORTANT: -c without -s should error (GNU rejects it)
+// GNU free: "free: -c requires -s option"
+// Currently: -c without -s silently displays once.
+// Expected: error exit code 2 with message about -s requirement.
+test "audit: free -c without -s should error" {
+    var stdout_buf = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stdout_buf.deinit(testing.allocator);
+    var stderr_buf = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stderr_buf.deinit(testing.allocator);
+
+    const args = [_][]const u8{ "-c", "3" };
+    const result = runFree(testing.allocator, &args, stdout_buf.writer(testing.allocator), stderr_buf.writer(testing.allocator));
+
+    // GNU exits 2 (misuse) when -c is given without -s
+    try testing.expectEqual(@as(u8, 2), result);
+    // stderr should mention -s requirement
+    try testing.expect(std.mem.indexOf(u8, stderr_buf.items, "-s") != null);
+}
+
+// IMPORTANT: -s short flag is hijacked by the si bool field
+// The argparse getShortFlag maps "si" -> 's' (first char), so -s
+// sets si=true instead of being parsed as --seconds. This means
+// `free -s 1` sets SI mode and treats "1" as a positional (error).
+// Expected: -s 1 should set seconds=1 for continuous display.
+test "audit: free -s should set seconds not si" {
+    var stdout_buf = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stdout_buf.deinit(testing.allocator);
+    var stderr_buf = try std.ArrayList(u8).initCapacity(testing.allocator, 0);
+    defer stderr_buf.deinit(testing.allocator);
+
+    // free -s 1 -c 1 should run continuous mode (1 second, 1 count)
+    // and succeed. With the bug, -s sets si=true, "1" becomes a
+    // positional, and we get "extra operand '1'" (exit 2).
+    const args = [_][]const u8{ "-s", "1", "-c", "1" };
+    const result = runFree(testing.allocator, &args, stdout_buf.writer(testing.allocator), stderr_buf.writer(testing.allocator));
+
+    // Should succeed (exit 0) — continuous mode with 1-second interval, 1 count
+    try testing.expectEqual(@as(u8, 0), result);
+    // Should produce output (at least one display)
+    try testing.expect(stdout_buf.items.len > 0);
+    try testing.expect(std.mem.indexOf(u8, stdout_buf.items, "Mem:") != null);
+}
diff --git a/tests/utilities/env_test.sh b/tests/utilities/env_test.sh
index ea44964..35ba461 100755
--- a/tests/utilities/env_test.sh
+++ b/tests/utilities/env_test.sh
@@ -321,4 +321,39 @@ test_env() {
         print_test_result "env -S produces no stderr warning" "FAIL" \
             "Expected no stderr, got: '$stderr_output'"
     fi
+
+    echo -e "${CYAN}Testing audit findings (wave 4)...${NC}"
+
+    # AUDIT: -0 with utility should be an error
+    # macOS spec: "Both -0 and utility may not be specified together."
+    "$binary" -0 echo hello 2>/dev/null
+    exit_code=$?
+    if [[ $exit_code -ne 0 ]]; then
+        print_test_result "env -0 with utility is rejected" "PASS"
+    else
+        print_test_result "env -0 with utility is rejected" "FAIL" \
+            "Expected non-zero exit, got: $exit_code"
+    fi
+
+    # AUDIT: Flags after NAME=VALUE should not be parsed as options
+    # After FOO=bar, -u should be treated as the command name
+    "$binary" -i FOO=bar -u FOO 2>/dev/null
+    exit_code=$?
+    if [[ $exit_code -eq 127 ]]; then
+        print_test_result "env flags after NAME=VALUE treated as command" "PASS"
+    else
+        print_test_result "env flags after NAME=VALUE treated as command" "FAIL" \
+            "Expected exit 127, got: $exit_code"
+    fi
+
+    # AUDIT: -P should not set PATH in child's environment
+    # env -i -P /usr/bin FOO=bar should print only FOO=bar, not PATH
+    local p_output
+    p_output=$("$binary" -i -P /usr/bin FOO=bar 2>/dev/null)
+    if [[ "$p_output" == "FOO=bar" ]]; then
+        print_test_result "env -P does not set child PATH" "PASS"
+    else
+        print_test_result "env -P does not set child PATH" "FAIL" \
+            "Expected only 'FOO=bar', got: '$p_output'"
+    fi
 }
diff --git a/tests/utilities/free_test.sh b/tests/utilities/free_test.sh
index a806ee5..015c5d3 100644
--- a/tests/utilities/free_test.sh
+++ b/tests/utilities/free_test.sh
@@ -118,4 +118,45 @@ test_free() {
     # Invalid flag exits with code 2
     test_command_exit_code "free invalid flag exits 2" 2 \
         "$binary" --invalid-flag
+
+    echo -e "${CYAN}Testing audit findings (wave 4)...${NC}"
+
+    # AUDIT: -w wide mode buffers column should show nonzero on Linux
+    if [[ "$(uname)" == "Linux" ]]; then
+        local wide_out
+        wide_out=$("$binary" -w -b 2>/dev/null)
+        # Extract the Mem: line
+        local mem_line
+        mem_line=$(echo "$wide_out" | grep "^Mem:")
+        # Parse the 5th numeric column (buffers)
+        local buffers_val
+        buffers_val=$(echo "$mem_line" | awk '{print $6}')
+        if [[ -n "$buffers_val" && "$buffers_val" -gt 0 ]] 2>/dev/null; then
+            print_test_result "free -w shows nonzero buffers" "PASS"
+        else
+            print_test_result "free -w shows nonzero buffers" "FAIL" \
+                "Buffers column is '$buffers_val', expected > 0. Line: '$mem_line'"
+        fi
+    fi
+
+    # AUDIT: -c without -s should error (GNU rejects it)
+    "$binary" -c 3 2>/dev/null
+    exit_code=$?
+    if [[ $exit_code -eq 2 ]]; then
+        print_test_result "free -c without -s exits 2" "PASS"
+    else
+        print_test_result "free -c without -s exits 2" "FAIL" \
+            "Expected exit 2, got: $exit_code"
+    fi
+
+    # AUDIT: -s should set seconds interval, not SI mode
+    # free -s 1 -c 1 should succeed (continuous mode, 1 iteration)
+    timeout 5 "$binary" -s 1 -c 1 >/dev/null 2>&1
+    exit_code=$?
+    if [[ $exit_code -eq 0 ]]; then
+        print_test_result "free -s 1 -c 1 sets seconds interval" "PASS"
+    else
+        print_test_result "free -s 1 -c 1 sets seconds interval" "FAIL" \
+            "Expected exit 0, got: $exit_code"
+    fi
 }