Skip to content

Commit 95adfb2

Browse files
committed
fix(cli): stop truncating config reads, cover + document the Nickel patchers
- config set-default / add-favorite read the user config into a fixed [16384]u8 buffer with a single read(), silently truncating (and then corrupting on rewrite) any config larger than 16 KiB. Switched both to readToEndAlloc(1 MiB). - Documented writeWithDefaultServer / writeWithFavorite as deliberately narrow, structure-aware patches of GSA's own generated user-config.ncl — explicitly not a Nickel parser — that pass unknown-shape input through unchanged rather than risk corruption. - Fixed the usage text: GSA_VERISIMDB_URL default printed http://[::1]:8090 but the code defaults to http://localhost:8090. - cli.zig had zero tests and was only built as an exe. Added a test-cli build step + 3 round-trip tests (set-default replace, favorite insert, unknown-shape passthrough) and wired test-cli + the fuzz seed run into CI. 162 tests green across six suites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TaWWedv6VQqeZaPvc94keN
1 parent 920058f commit 95adfb2

3 files changed

Lines changed: 131 additions & 9 deletions

File tree

.github/workflows/cross-platform.yml

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,3 +72,11 @@ jobs:
7272
- name: Behavioral Tests (mock servers)
7373
working-directory: src/interface/ffi
7474
run: zig build test-behavioral
75+
76+
- name: CLI Tests
77+
working-directory: src/interface/ffi
78+
run: zig build test-cli
79+
80+
- name: Fuzz seed corpus
81+
working-directory: src/interface/ffi
82+
run: zig build fuzz

src/interface/ffi/build.zig

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,25 @@ pub fn build(b: *std.Build) void {
226226
const fuzz_step = b.step("fuzz", "Run config-parser fuzz harnesses (append --fuzz for continuous fuzzing)");
227227
fuzz_step.dependOn(&run_fuzz_tests.step);
228228

229+
// ---------------------------------------------------------------
230+
// CLI tests — the gsa executable's own unit tests (config helpers).
231+
// The exe is not otherwise covered by any `test` step.
232+
// ---------------------------------------------------------------
233+
const cli_mod = b.createModule(.{
234+
.root_source_file = b.path("src/cli.zig"),
235+
.target = target,
236+
.optimize = optimize,
237+
.link_libc = true,
238+
});
239+
240+
const cli_tests = b.addTest(.{
241+
.root_module = cli_mod,
242+
});
243+
244+
const run_cli_tests = b.addRunArtifact(cli_tests);
245+
const cli_step = b.step("test-cli", "Run the gsa CLI's unit tests");
246+
cli_step.dependOn(&run_cli_tests.step);
247+
229248
// ---------------------------------------------------------------
230249
// Benchmarks — micro-benchmark executable (prints to stderr)
231250
// ---------------------------------------------------------------

src/interface/ffi/src/cli.zig

Lines changed: 104 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -374,9 +374,16 @@ fn cmdConfigSetDefault(host: []const u8, port: u16, out: std.fs.File) void {
374374
};
375375
defer file.close();
376376

377-
var existing: [16384]u8 = undefined;
378-
const n = file.read(&existing) catch 0;
379-
const existing_content = existing[0..n];
377+
// Read the whole file — a fixed 16 KiB buffer silently truncated larger
378+
// configs (e.g. many favorites), corrupting the rewrite.
379+
const existing_content = file.readToEndAlloc(std.heap.c_allocator, 1024 * 1024) catch |err| {
380+
const err_f = std.fs.File.stderr();
381+
err_f.writeAll("✗ Failed to read config: ") catch {};
382+
err_f.writeAll(@errorName(err)) catch {};
383+
err_f.writeAll("\n") catch {};
384+
std.process.exit(1);
385+
};
386+
defer std.heap.c_allocator.free(existing_content);
380387

381388
// Simple string replacement for default server
382389
// In a real implementation, you'd use a proper Nickel parser
@@ -422,9 +429,16 @@ fn cmdConfigAddFavorite(name: []const u8, host: []const u8, port: u16, out: std.
422429
};
423430
defer file.close();
424431

425-
var existing: [16384]u8 = undefined;
426-
const n = file.read(&existing) catch 0;
427-
const existing_content = existing[0..n];
432+
// Read the whole file — a fixed 16 KiB buffer silently truncated larger
433+
// configs (e.g. many favorites), corrupting the rewrite.
434+
const existing_content = file.readToEndAlloc(std.heap.c_allocator, 1024 * 1024) catch |err| {
435+
const err_f = std.fs.File.stderr();
436+
err_f.writeAll("✗ Failed to read config: ") catch {};
437+
err_f.writeAll(@errorName(err)) catch {};
438+
err_f.writeAll("\n") catch {};
439+
std.process.exit(1);
440+
};
441+
defer std.heap.c_allocator.free(existing_content);
428442

429443
// Add favorite to the favorites list
430444

@@ -468,7 +482,15 @@ fn cmdConfigListFavorites(out: std.fs.File) void {
468482
out.writeAll("Use `gsa config show` to view the full config.\n") catch {};
469483
}
470484

471-
/// Helper: Write config content with replaced default server to a file.
485+
/// Helper: rewrite the `host = ...` / `port = ...` lines of the generated
486+
/// user-config.ncl with a new default server.
487+
///
488+
/// This is a deliberately narrow, structure-aware text patch — NOT a Nickel
489+
/// parser. It relies on the shape GSA itself emits (`gsa config init` from the
490+
/// bundled template): top-level `host = "..."` and `port = N` lines. On any
491+
/// unexpected shape (either marker missing) it writes the input back unchanged
492+
/// rather than risk corrupting a hand-edited file. A full Nickel editor is out
493+
/// of scope for the CLI. Round-tripped by the tests at the bottom of this file.
472494
fn writeWithDefaultServer(existing: []const u8, host: []const u8, port: u16, out_file: std.fs.File) !void {
473495
const host_start = std.mem.indexOf(u8, existing, "host = ") orelse {
474496
_ = try out_file.write(existing);
@@ -496,7 +518,10 @@ fn writeWithDefaultServer(existing: []const u8, host: []const u8, port: u16, out
496518
}
497519
}
498520

499-
/// Helper: Write config content with a new favorite appended to a file.
521+
/// Helper: append a favorite entry before the closing `]` of the favorites
522+
/// list. Same philosophy as writeWithDefaultServer — a structure-aware patch of
523+
/// GSA's own generated file, not a Nickel parser; on a missing `]` it writes the
524+
/// input back unchanged. Round-tripped by the tests at the bottom of this file.
500525
fn writeWithFavorite(existing: []const u8, name: []const u8, host: []const u8, port: u16, out_file: std.fs.File) !void {
501526
const fav_end = std.mem.indexOf(u8, existing, "]") orelse {
502527
_ = try out_file.write(existing);
@@ -629,9 +654,79 @@ fn printUsage(out: std.fs.File) void {
629654
\\ help Show this help
630655
\\
631656
\\ Environment:
632-
\\ GSA_VERISIMDB_URL VeriSimDB endpoint (default: http://[::1]:8090)
657+
\\ GSA_VERISIMDB_URL VeriSimDB endpoint (default: http://localhost:8090)
633658
\\ GSA_PROFILES_DIR Profiles directory (default: ./profiles)
634659
\\
635660
\\
636661
) catch {};
637662
}
663+
664+
// ── Tests ────────────────────────────────────────────────────────────────────
665+
666+
test "writeWithDefaultServer replaces host/port and preserves the rest" {
667+
var tmp = std.testing.tmpDir(.{});
668+
defer tmp.cleanup();
669+
const existing =
670+
\\{
671+
\\ default_server = {
672+
\\ host = "old.example.com",
673+
\\ port = 12345,
674+
\\ },
675+
\\ favorites = [],
676+
\\}
677+
;
678+
{
679+
const f = try tmp.dir.createFile("c.ncl", .{});
680+
defer f.close();
681+
try writeWithDefaultServer(existing, "new.host", 25565, f);
682+
}
683+
const rf = try tmp.dir.openFile("c.ncl", .{});
684+
defer rf.close();
685+
const content = try rf.readToEndAlloc(std.testing.allocator, 1 << 16);
686+
defer std.testing.allocator.free(content);
687+
688+
try std.testing.expect(std.mem.indexOf(u8, content, "host = \"new.host\"") != null);
689+
try std.testing.expect(std.mem.indexOf(u8, content, "port = 25565") != null);
690+
try std.testing.expect(std.mem.indexOf(u8, content, "old.example.com") == null);
691+
try std.testing.expect(std.mem.indexOf(u8, content, "favorites = []") != null);
692+
}
693+
694+
test "writeWithFavorite inserts an entry before the closing bracket" {
695+
var tmp = std.testing.tmpDir(.{});
696+
defer tmp.cleanup();
697+
const existing =
698+
\\{
699+
\\ favorites = [
700+
\\ ],
701+
\\}
702+
;
703+
{
704+
const f = try tmp.dir.createFile("c.ncl", .{});
705+
defer f.close();
706+
try writeWithFavorite(existing, "myfav", "10.0.0.1", 27015, f);
707+
}
708+
const rf = try tmp.dir.openFile("c.ncl", .{});
709+
defer rf.close();
710+
const content = try rf.readToEndAlloc(std.testing.allocator, 1 << 16);
711+
defer std.testing.allocator.free(content);
712+
713+
try std.testing.expect(std.mem.indexOf(u8, content, "name = \"myfav\"") != null);
714+
try std.testing.expect(std.mem.indexOf(u8, content, "host = \"10.0.0.1\"") != null);
715+
try std.testing.expect(std.mem.indexOf(u8, content, "port = 27015") != null);
716+
}
717+
718+
test "config helpers preserve unknown-shape input unchanged (no corruption)" {
719+
var tmp = std.testing.tmpDir(.{});
720+
defer tmp.cleanup();
721+
const weird = "this is not the generated config at all\n";
722+
{
723+
const f = try tmp.dir.createFile("c.ncl", .{});
724+
defer f.close();
725+
try writeWithDefaultServer(weird, "x", 1, f);
726+
}
727+
const rf = try tmp.dir.openFile("c.ncl", .{});
728+
defer rf.close();
729+
const content = try rf.readToEndAlloc(std.testing.allocator, 1 << 16);
730+
defer std.testing.allocator.free(content);
731+
try std.testing.expectEqualStrings(weird, content);
732+
}

0 commit comments

Comments
 (0)