commit 901ed9b95d701f2ea92f2192810727a510191ff0 from: mtmn date: Mon Sep 14 19:39:01 2026 UTC fix fzf escape, limit cache sizes and file permissions commit - d294179de2c1ba95d8ecac7edb510c2ef8b659e2 commit + 901ed9b95d701f2ea92f2192810727a510191ff0 blob - 23f01221c0f9da9f7fa8b47c9deb04548dc0d47f blob + 33160d1c8fe4ff92174d0103c610815c96dde818 --- src/db.zig +++ src/db.zig @@ -45,22 +45,33 @@ pub const Db = struct { std.Io.Dir.cwd().createDirPath(io.rt(), base_path) catch |err| { if (err != error.PathAlreadyExists) return err; }; + restrictDirPerms(base_path); for (&[_][]const u8{ dirs_path, files_path }) |p| { var f = std.Io.Dir.cwd().openFile(io.rt(), p, .{ .mode = .read_write }) catch |err| { if (err == error.FileNotFound) { - const created = try std.Io.Dir.cwd().createFile(io.rt(), p, .{ .truncate = false }); + const created = try std.Io.Dir.cwd().createFile(io.rt(), p, .{ .truncate = false, .permissions = .fromMode(0o600) }); created.close(io.rt()); continue; } return err; }; + // Best effort: tighten perms on pre-existing files too. + f.setPermissions(io.rt(), .fromMode(0o600)) catch {}; f.close(io.rt()); } return res; } + // History logs hold absolute paths with timestamps. Restrict to owner on + // a best-effort basis; failures never fail init (read-only parent, ACLs). + fn restrictDirPerms(path: []const u8) void { + var dir = std.Io.Dir.cwd().openDir(io.rt(), path, .{ .iterate = true }) catch return; + defer dir.close(io.rt()); + dir.setPermissions(io.rt(), .fromMode(0o700)) catch {}; + } + pub fn deinit(self: *Db) void { self.allocator.free(self.base_path); self.allocator.free(self.dirs_path); @@ -264,7 +275,7 @@ pub const Db = struct { const duplicates = total_entries - lines.items.len - missing_entries - inaccessible_entries; io.warn(" Entries: {d} total, {d} unique, {d} missing, {d} inaccessible, {d} duplicates removed\n", .{ total_entries, lines.items.len, missing_entries, inaccessible_entries, duplicates }); - const write_file = try std.Io.Dir.cwd().createFile(io.rt(), path, .{ .truncate = true }); + const write_file = try std.Io.Dir.cwd().createFile(io.rt(), path, .{ .truncate = true, .permissions = .fromMode(0o600) }); defer write_file.close(io.rt()); var write_buf: [4096]u8 = undefined; blob - 0db24b50ba05f1f927ea8f9517cf7abf4ef9f8ac blob + 258bee76c6beb8aae2d1ef555f6ae553207ff677 --- src/fzf.zig +++ src/fzf.zig @@ -46,6 +46,7 @@ fn gotoFile(allocator: std.mem.Allocator, db: ?*hist.D if (line) |l| { const line_arg = try std.fmt.allocPrint(allocator, "+{s}", .{l}); + defer allocator.free(line_arg); return std.process.replace(io.rt(), .{ .argv = &.{ editor, line_arg, base_name } }); } @@ -136,7 +137,7 @@ fn runFzfPicker(allocator: std.mem.Allocator, fzf_opts if (child.stdout) |stdout| { var buffer: [4096]u8 = undefined; var sr = stdout.reader(io.rt(), &buffer); - stdout_data = try sr.interface.allocRemaining(allocator, .unlimited); + stdout_data = try sr.interface.allocRemaining(allocator, .limited(64 * 1024 * 1024)); } const term = try child.wait(io.rt()); @@ -265,28 +266,46 @@ pub fn recentFile(allocator: std.mem.Allocator, db: *h try gotoFile(allocator, db, selected, null, config); } -pub fn grep(allocator: std.mem.Allocator, db: *hist.Db, config: *const cfg.Config) !void { - const initial_query = ""; +// fzf `--bind change:reload:...` runs via shell. Patterns come from config, +// so single-quote them safely: `'` becomes `'\''` inside outer `'...'`. +fn appendShellEscapedContent(buf: *std.ArrayListUnmanaged(u8), allocator: std.mem.Allocator, s: []const u8) !void { + for (s) |c| { + if (c == '\'') { + try buf.appendSlice(allocator, "'\\''"); + } else { + try buf.append(allocator, c); + } + } +} +fn buildRgReloadCommand(allocator: std.mem.Allocator, ignored_patterns: []const []const u8) ![]u8 { var rg_cmd = std.ArrayListUnmanaged(u8).empty; - defer rg_cmd.deinit(allocator); + errdefer rg_cmd.deinit(allocator); try rg_cmd.appendSlice(allocator, "change:reload:rg --column --line-number --no-heading --color=always --smart-case"); - for (config.ignored_patterns) |pattern| { + for (ignored_patterns) |pattern| { try rg_cmd.appendSlice(allocator, " --glob '!"); - try rg_cmd.appendSlice(allocator, pattern); + try appendShellEscapedContent(&rg_cmd, allocator, pattern); try rg_cmd.appendSlice(allocator, "/*'"); try rg_cmd.appendSlice(allocator, " --glob '!"); - try rg_cmd.appendSlice(allocator, pattern); + try appendShellEscapedContent(&rg_cmd, allocator, pattern); try rg_cmd.appendSlice(allocator, "'"); } try rg_cmd.appendSlice(allocator, " {q} || true"); + return rg_cmd.toOwnedSlice(allocator); +} +pub fn grep(allocator: std.mem.Allocator, db: *hist.Db, config: *const cfg.Config) !void { + const initial_query = ""; + + const rg_reload = try buildRgReloadCommand(allocator, config.ignored_patterns); + defer allocator.free(rg_reload); + var fzf = try Fzf.init(allocator, config.fzf_opts, &[_][]const u8{ "--disabled", "--query", initial_query, "--bind", - rg_cmd.items, + rg_reload, "--delimiter", ":", "--preview", @@ -308,7 +327,7 @@ pub fn grep(allocator: std.mem.Allocator, db: *hist.Db if (child.stdout) |stdout| { var buffer: [4096]u8 = undefined; var sr = stdout.reader(io.rt(), &buffer); - stdout_data = try sr.interface.allocRemaining(allocator, .unlimited); + stdout_data = try sr.interface.allocRemaining(allocator, .limited(64 * 1024 * 1024)); } const term = try child.wait(io.rt()); @@ -436,6 +455,19 @@ fn collectEntries( } } +// Cached entries come from unauthenticated localhost memcached. Never trust +// them blindly: reject absolute paths, `..` escapes, empty/NUL paths. +fn isSafeCachedPath(p: []const u8) bool { + if (p.len == 0) return false; + if (std.mem.indexOfScalar(u8, p, 0) != null) return false; + if (std.fs.path.isAbsolute(p)) return false; + var it = std.mem.splitScalar(u8, p, std.fs.path.sep); + while (it.next()) |comp| { + if (std.mem.eql(u8, comp, "..")) return false; + } + return true; +} + fn runFzfWithWalker(allocator: std.mem.Allocator, db: *hist.Db, kind: std.Io.File.Kind, config: *const cfg.Config, depth: ?usize) !void { const is_dir = kind == .directory; const max_depth = depth orelse config.max_depth; @@ -475,18 +507,33 @@ fn runFzfWithWalker(allocator: std.mem.Allocator, db: const mtime = @divTrunc(max_dir_mtime, std.time.ns_per_s); cache_key = try std.fmt.allocPrint(allocator, "magdalena:look:{s}:{x}:{d}:{d}", .{ if (is_dir) "dir" else "file", hash, mtime, max_depth }); if (cache_key) |key| { - const cached_data = try m.get(key); - if (cached_data) |data| { + // Cache lives in unauthenticated localhost memcached. Any + // failure or corruption is a miss, never fatal. + const maybe_data: ?[]u8 = m.get(key) catch |err| blk: { + io.warn("ignoring cache read failure: {}\n", .{err}); + break :blk null; + }; + if (maybe_data) |data| { defer allocator.free(data); - const parsed = try std.json.parseFromSlice([]Entry, allocator, data, .{}); - defer parsed.deinit(); - for (parsed.value) |e| { - try entries.append(allocator, .{ - .path = try allocator.dupe(u8, e.path), - .mtime = e.mtime, - }); + const maybe_parsed: ?std.json.Parsed([]Entry) = std.json.parseFromSlice([]Entry, allocator, data, .{}) catch |err| blk: { + io.warn("ignoring corrupt cache entry: {}\n", .{err}); + break :blk null; + }; + if (maybe_parsed) |parsed| { + defer parsed.deinit(); + var valid: usize = 0; + for (parsed.value) |e| { + if (!isSafeCachedPath(e.path)) continue; + try entries.append(allocator, .{ + .path = try allocator.dupe(u8, e.path), + .mtime = e.mtime, + }); + valid += 1; + } + // Parseable but fully bogus entries fall through to a + // fresh walk, which also overwrites the poisoned key. + if (valid > 0 or parsed.value.len == 0) cache_hit = true; } - cache_hit = true; } } } @@ -501,7 +548,9 @@ fn runFzfWithWalker(allocator: std.mem.Allocator, db: if (cache_key) |key| { const json_data = try std.json.Stringify.valueAlloc(allocator, entries.items, .{}); defer allocator.free(json_data); - try m.set(key, json_data, 3600); + m.set(key, json_data, 3600) catch |err| { + io.warn("failed to write cache: {}\n", .{err}); + }; } } } @@ -557,3 +606,32 @@ test "shouldSkip: partial component is not a match" { test "shouldSkip: empty pattern set never skips" { try testing.expect(!shouldSkip("a/b/c", &.{})); } + +test "isSafeCachedPath: accepts plain relative paths" { + try testing.expect(isSafeCachedPath("src/main.zig")); + try testing.expect(isSafeCachedPath("./a/b")); +} + +test "isSafeCachedPath: rejects escapes and junk" { + try testing.expect(!isSafeCachedPath("")); + try testing.expect(!isSafeCachedPath("/etc/passwd")); + try testing.expect(!isSafeCachedPath("a/../../etc/passwd")); + try testing.expect(!isSafeCachedPath("..")); + try testing.expect(!isSafeCachedPath("a\x00b")); +} + +test "buildRgReloadCommand: plain patterns stay single-quoted" { + const ignored = [_][]const u8{ "node_modules", ".git" }; + const cmd = try buildRgReloadCommand(testing.allocator, &ignored); + defer testing.allocator.free(cmd); + try testing.expect(std.mem.indexOf(u8, cmd, "--glob '!node_modules/*'") != null); + try testing.expect(std.mem.indexOf(u8, cmd, "--glob '!.git'") != null); +} + +test "buildRgReloadCommand: single quote cannot break out" { + const ignored = [_][]const u8{"a'b"}; + const cmd = try buildRgReloadCommand(testing.allocator, &ignored); + defer testing.allocator.free(cmd); + try testing.expect(std.mem.indexOf(u8, cmd, "'!a'\\''b/*'") != null); + try testing.expect(std.mem.indexOf(u8, cmd, "'!a'\\''b'") != null); +} blob - 1939677accd3af9aeb9cd4ca98dedd28e2406993 blob + cbb414bde50183b33c6ff57c94a3f2bb1812b46a --- src/memcached.zig +++ src/memcached.zig @@ -56,10 +56,16 @@ pub const Memcached = struct { const chunk_size = 900 * 1024; const chunk_prefix = "\x00chunked:"; + // Upper bound for any single value we will send or accept. Answers come + // from an unauthenticated localhost server, so never allocate blindly + // from a peer-supplied length. + pub const max_value_bytes = 64 * 1024 * 1024; - // Large values are split into chunk keys `{key}:{i}`. Overwriting a chunked - // key with a smaller value leaves old chunks as orphans. + // Large values are split into chunk keys `{key}:{total}:{i}`. Namespacing + // chunks by total length means overwriting with a smaller value cannot + // alias stale chunks from a previous larger value; leftovers expire via TTL. pub fn set(self: *Memcached, key: []const u8, value: []const u8, exptime: u32) !void { + if (value.len > max_value_bytes) return error.ValueTooLarge; if (value.len <= chunk_size) { try self.sendSet(key, value, exptime); return; @@ -75,7 +81,7 @@ pub const Memcached = struct { const start = i * chunk_size; const end = @min((i + 1) * chunk_size, value.len); var chunk_key_buf: [512]u8 = undefined; - const chunk_key = try std.fmt.bufPrint(&chunk_key_buf, "{s}:{d}", .{ key, i }); + const chunk_key = try std.fmt.bufPrint(&chunk_key_buf, "{s}:{d}:{d}", .{ key, value.len, i }); try self.sendSet(chunk_key, value[start..end], exptime); } } @@ -137,6 +143,7 @@ pub const Memcached = struct { const rest = line[3..]; const space_idx = std.mem.indexOfScalar(u8, rest, ' ') orelse rest.len; const size = try std.fmt.parseInt(usize, rest[0..space_idx], 10); + if (size > max_value_bytes) return error.ValueTooLarge; const data = try self.allocator.alloc(u8, size); errdefer self.allocator.free(data); @@ -165,6 +172,9 @@ pub const Memcached = struct { const num_chunks = try std.fmt.parseInt(usize, num_chunks_str, 10); const total_size = try std.fmt.parseInt(usize, total_size_str, 10); + if (num_chunks == 0) return error.InvalidCacheData; + if (total_size > max_value_bytes) return error.ValueTooLarge; + if (num_chunks != (total_size + chunk_size - 1) / chunk_size) return error.InvalidCacheData; const result = try self.allocator.alloc(u8, total_size); errdefer self.allocator.free(result); @@ -172,7 +182,7 @@ pub const Memcached = struct { var i: usize = 0; while (i < num_chunks) : (i += 1) { var chunk_key_buf: [512]u8 = undefined; - const chunk_key = try std.fmt.bufPrint(&chunk_key_buf, "{s}:{d}", .{ key, i }); + const chunk_key = try std.fmt.bufPrint(&chunk_key_buf, "{s}:{d}:{d}", .{ key, total_size, i }); var cmd_buf: [512]u8 = undefined; const cmd = try std.fmt.bufPrint(&cmd_buf, "mg {s} v\r\n", .{chunk_key});