From bd6bc1d161085d6d3e7d8bb453c891cbeaf4066b Mon Sep 17 00:00:00 2001 From: Emil Lerch Date: Wed, 1 Jul 2026 10:49:02 -0700 Subject: [PATCH] prefer ZFIN_HOME for .env, match portfolio loading paradigm --- AGENTS.md | 2 +- docs/reference/config/environment.md | 8 +- src/Config.zig | 162 +++++++++++++++++++++++---- 3 files changed, 146 insertions(+), 26 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5c42b0b..35eea4d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -775,7 +775,7 @@ command. - **The `color` parameter flows through everything.** CLI commands accept a `color: bool` parameter. Don't use ANSI escapes unconditionally - always gate on the `color` flag. -- **Portfolio auto-detection.** Both CLI and TUI auto-load `portfolio.srf` from cwd if no explicit path is given. If not found in cwd, falls back to `$ZFIN_HOME/portfolio.srf`. `watchlist.srf` and `.env` follow the same cascade. `metadata.srf` and `accounts.srf` are loaded from the same directory as the resolved portfolio file. +- **Portfolio auto-detection.** Both CLI and TUI resolve `portfolio*.srf` the same way, and it is ZFIN_HOME-exclusive when set: if `$ZFIN_HOME` is set, only that directory is searched (cwd is NOT a fallback - a project directory that incidentally ships a `portfolio.srf` must not shadow the user's canonical data); if `$ZFIN_HOME` is unset, cwd is searched. An explicit `-p` path resolves the same exclusive way. `watchlist.srf` and the `.env` file follow the same rule (`.env` adds process environment variables as a higher-priority tier on top). `metadata.srf` and `accounts.srf` are loaded from the same directory as the resolved portfolio file. - **`transaction_log.srf` is a sibling file.** Optional. Lives next to `portfolio.srf` / `accounts.srf`. Holds user-declared `transfer::` records so the contributions pipeline can tell internal account-to-account movement apart from real external contributions. Only `type::cash` is wired in v1 - `type::in_kind` parses but is rejected downstream. Missing file -> matcher is a no-op. See `REPORT.md` section 5 "Transfer log" for the user-facing guide. diff --git a/docs/reference/config/environment.md b/docs/reference/config/environment.md index 84e3c47..140d8a9 100644 --- a/docs/reference/config/environment.md +++ b/docs/reference/config/environment.md @@ -1,9 +1,11 @@ # Environment variables zfin is configured through environment variables. Set them in your -shell, or put them in a `.env` file. The `.env` file is searched first -in the binary's parent directory, then in the current directory; any -value set in the real environment is also honored. +shell, or put them in a `.env` file. Values set in the real environment +always win; a `.env` file supplies anything not already set. The `.env` +file is resolved the same way as your portfolio: when `ZFIN_HOME` is +set, only `$ZFIN_HOME/.env` is read (the current directory is not also +searched); otherwise the `.env` in the current directory is used. ```bash # .env diff --git a/src/Config.zig b/src/Config.zig index 7c5ce47..7593510 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -6,9 +6,13 @@ //! `Config.ResolvedPath`, `Config.default_portfolio_filename`, etc.). //! //! Configuration sources, in priority order: -//! 1. Process environment variables -//! 2. `.env` file in the current working directory -//! 3. `.env` file in `$ZFIN_HOME` +//! 1. Process environment variables (always win). +//! 2. A single `.env` file, selected the same way portfolio files are +//! resolved (see `resolveUserFile`): when `$ZFIN_HOME` is set, ONLY +//! `$ZFIN_HOME/.env` is read - the cwd `.env` is not consulted, so +//! running from a project directory that ships its own `.env` can't +//! shadow the user's canonical secrets. When `$ZFIN_HOME` is unset, +//! the cwd `.env` is used. //! //! Cache directory defaults follow XDG: `$ZFIN_CACHE_DIR` > `$XDG_CACHE_HOME/zfin` //! > `$HOME/.cache/zfin`. @@ -19,7 +23,9 @@ const builtin = @import("builtin"); const EnvMap = std.StringHashMap([]const u8); /// Default pattern for the portfolio file when no explicit -p/--portfolio -/// is provided. Looked up via `resolveUserFiles` (cwd -> ZFIN_HOME). The +/// is provided. Looked up via `resolveUserFiles`, which is +/// ZFIN_HOME-exclusive when ZFIN_HOME is set (cwd is not consulted) and +/// cwd-relative otherwise. The /// `*` is intentional - multiple files matching `portfolio*.srf` are all /// loaded and union-merged. A user with just one `portfolio.srf` is /// unaffected (the glob still matches that single file). Every command @@ -75,10 +81,20 @@ cache_dir: []const u8, cache_dir_owned: bool = false, // true when cache_dir was allocated via path.join zfin_home: ?[]const u8 = null, allocator: ?std.mem.Allocator = null, -/// Raw .env file contents (keys/values in env_map point into this). +/// Raw bytes of the cwd `.env` file (when one exists). Keys/values in the +/// cwd `env_map` point into this; it also backs `zfin_home` when ZFIN_HOME +/// is declared in the cwd `.env`, so it is kept alive for the Config's +/// lifetime even after `switchToHomeEnv` makes `$ZFIN_HOME/.env` the +/// active source. env_buf: ?[]const u8 = null, -/// Parsed KEY=VALUE pairs from .env file (fallback when process env is missing a key). +/// Parsed KEY=VALUE pairs from the active `.env` file, consulted after the +/// process environment (see `resolve`). `fromEnv` points this at +/// `$ZFIN_HOME/.env` when ZFIN_HOME is set, otherwise the cwd `.env`. env_map: ?EnvMap = null, +/// Raw bytes of `$ZFIN_HOME/.env` once it becomes the active source (see +/// `switchToHomeEnv`). Held separately from `env_buf` because that field +/// may still be pinning the cwd `.env` bytes that back `zfin_home`. +env_buf_home: ?[]const u8 = null, /// Process-level environment variable map (from Juicy Main). First-priority /// lookup source before falling back to the .env file. environ_map: ?*const std.process.Environ.Map = null, @@ -87,15 +103,18 @@ environ_map: ?*const std.process.Environ.Map = null, pub fn fromEnv(io: std.Io, allocator: std.mem.Allocator, environ_map: *const std.process.Environ.Map) @This() { var self = @This(){ - // SAFETY: assigned unconditionally below (line ~95) from - // either ZFIN_CACHE_DIR or the XDG fallback before this - // function returns, so callers never observe `undefined`. + // SAFETY: assigned unconditionally below (the `cache_dir = + // env_cache orelse ...` block) from either ZFIN_CACHE_DIR or the + // XDG fallback before this function returns, so callers never + // observe `undefined`. .cache_dir = undefined, .allocator = allocator, .environ_map = environ_map, }; - // Try loading .env file from the current working directory + // Load the cwd .env first. It's both a candidate config source (used + // when ZFIN_HOME is unset) and the place ZFIN_HOME itself may be + // declared, so it must be parsed before we resolve ZFIN_HOME. self.env_buf = std.Io.Dir.cwd().readFileAlloc(io, ".env", allocator, .limited(4096)) catch null; if (self.env_buf) |buf| { self.env_map = parseEnvFile(allocator, buf); @@ -103,18 +122,15 @@ pub fn fromEnv(io: std.Io, allocator: std.mem.Allocator, environ_map: *const std self.zfin_home = self.resolve("ZFIN_HOME"); - // Try loading .env file from ZFIN_HOME as well (cwd .env takes priority) - if (self.env_buf == null) { - if (self.zfin_home) |home| { - const env_path = std.fs.path.join(allocator, &.{ home, ".env" }) catch null; - if (env_path) |p| { - defer allocator.free(p); - self.env_buf = std.Io.Dir.cwd().readFileAlloc(io, p, allocator, .limited(4096)) catch null; - if (self.env_buf) |buf| { - self.env_map = parseEnvFile(allocator, buf); - } - } - } + // ZFIN_HOME is authoritative when set, mirroring portfolio-file + // resolution (see resolveUserFile): the config lives in + // $ZFIN_HOME/.env and the cwd .env is NOT consulted, so running from + // a project directory that ships its own .env can't shadow the + // user's canonical secrets. switchToHomeEnv discards the cwd map and + // reads $ZFIN_HOME/.env instead; if that file is absent the active + // map is left null (deliberately no cwd fallback). + if (self.zfin_home) |home| { + self.switchToHomeEnv(io, allocator, home); } self.twelvedata_key = self.resolve("TWELVEDATA_API_KEY"); @@ -145,6 +161,32 @@ pub fn fromEnv(io: std.Io, allocator: std.mem.Allocator, environ_map: *const std return self; } +/// Switch the active `.env` source to `$ZFIN_HOME/.env`, discarding the +/// cwd `.env` map loaded earlier in `fromEnv`. Implements the +/// ZFIN_HOME-exclusive rule that mirrors portfolio-file resolution (see +/// `resolveUserFile`): once ZFIN_HOME is set, only `$ZFIN_HOME/.env` +/// contributes and the cwd `.env` is ignored. +/// +/// `env_buf` (the cwd `.env` bytes) is intentionally left allocated - +/// `self.zfin_home` may be a slice into it. It is released by `deinit`. +/// If `$ZFIN_HOME/.env` is missing or unreadable, the active map is left +/// null: there is deliberately no fallback to the cwd `.env`. +fn switchToHomeEnv(self: *@This(), io: std.Io, allocator: std.mem.Allocator, home: []const u8) void { + if (self.env_map) |*m| { + var cwd_map = m.*; + cwd_map.deinit(); + self.env_map = null; + } + const env_path = std.fs.path.join(allocator, &.{ home, ".env" }) catch null; + if (env_path) |p| { + defer allocator.free(p); + self.env_buf_home = std.Io.Dir.cwd().readFileAlloc(io, p, allocator, .limited(4096)) catch null; + if (self.env_buf_home) |buf| { + self.env_map = parseEnvFile(allocator, buf); + } + } +} + pub fn deinit(self: *@This()) void { if (self.allocator) |a| { if (self.env_map) |*m| { @@ -152,6 +194,7 @@ pub fn deinit(self: *@This()) void { map.deinit(); } if (self.env_buf) |buf| a.free(buf); + if (self.env_buf_home) |buf| a.free(buf); if (self.cache_dir_owned) { a.free(self.cache_dir); } @@ -680,6 +723,81 @@ test "resolve: environ_map=null and env_map=null returns null" { try testing.expect(c.resolve("ANYTHING") == null); } +test "switchToHomeEnv: ZFIN_HOME .env replaces the cwd .env (exclusive)" { + // Regression for "unauthorized until I exported ZFIN_SERVER_API_KEY": + // running from a directory with its own .env used to shadow the + // ZFIN_HOME .env entirely. Now ZFIN_HOME is authoritative when set + // (mirrors resolveUserFile): the cwd .env is discarded and only + // $ZFIN_HOME/.env contributes. + const allocator = testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + try tmp.dir.writeFile(io, .{ + .sub_path = ".env", + .data = "ZFIN_SERVER_API_KEY=home-secret\nSHARED=home\n", + }); + var home_buf: [std.fs.max_path_bytes]u8 = undefined; + const home_len = try tmp.dir.realPath(io, &home_buf); + const home = home_buf[0..home_len]; + + // Simulate fromEnv's state right after loading a cwd .env: a map with + // a key unique to cwd and a key that also exists in the home .env. + var cwd_map = EnvMap.init(allocator); + try cwd_map.put("CWD_ONLY", "cwd-only"); + try cwd_map.put("SHARED", "cwd"); + + var c: @This() = .{ + .cache_dir = "/tmp", + .allocator = allocator, + .env_map = cwd_map, + .zfin_home = home, + }; + defer c.deinit(); + + c.switchToHomeEnv(io, allocator, home); + + // cwd-only key is gone: the cwd map was discarded, not merged. + try testing.expect(c.resolve("CWD_ONLY") == null); + // The key that lived only in the ZFIN_HOME .env now resolves - this + // is exactly the bug that was being fixed. + try testing.expectEqualStrings("home-secret", c.resolve("ZFIN_SERVER_API_KEY").?); + // A key present in both reflects the ZFIN_HOME .env, not cwd. + try testing.expectEqualStrings("home", c.resolve("SHARED").?); +} + +test "switchToHomeEnv: missing ZFIN_HOME .env leaves no cwd fallback" { + // ZFIN_HOME set but no .env there: the cwd map is still discarded and + // nothing takes its place (process env only). Mirrors resolveUserFile + // returning null rather than falling back to cwd. + const allocator = testing.allocator; + const io = std.testing.io; + + var tmp = std.testing.tmpDir(.{}); + defer tmp.cleanup(); + // Intentionally do NOT write a .env into the tmp (ZFIN_HOME) dir. + var home_buf: [std.fs.max_path_bytes]u8 = undefined; + const home_len = try tmp.dir.realPath(io, &home_buf); + const home = home_buf[0..home_len]; + + var cwd_map = EnvMap.init(allocator); + try cwd_map.put("CWD_ONLY", "cwd-only"); + + var c: @This() = .{ + .cache_dir = "/tmp", + .allocator = allocator, + .env_map = cwd_map, + .zfin_home = home, + }; + defer c.deinit(); + + c.switchToHomeEnv(io, allocator, home); + + try testing.expect(c.env_map == null); + try testing.expect(c.resolve("CWD_ONLY") == null); +} + // ── Glob tests ──────────────────────────────────────────────── test "isGlobPattern: detects metacharacters" {