From 563a567fac2a3dff8b907781ca42daaac98d9682 Mon Sep 17 00:00:00 2001 From: Shashank Shekhar Singh Date: Wed, 5 Aug 2026 00:13:33 +0530 Subject: [PATCH] Read `.env` from the working directory, as `grapharc.toml` already is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `find_env_file` walked up parent directories to `/`, while the config layer next door refuses exactly that on principle — so the file that *spends money* was discovered more eagerly than the one that *constrains* a run. A run started in a scratch subdirectory picked up an `OPENROUTER_API_KEY` from any ancestor: a `.env` in `$HOME` billed every user's experiment on a shared box to that key, and since `redact()` is the only thing that ever prints a key, nothing said which file paid. The start directory (default: cwd) is now the only directory consulted. The signature and the None-when-absent contract are unchanged, so `get_secret` and the four backend accessors needed no edit, and neither escape hatch moved: a real environment variable still beats any file, and `env_file=` still names a file anywhere. No search boundary replaces the walk — stopping at a git root would still be an upward search. Behaviour change: a parent-directory `.env` stops being read. It is called out in the README's limits list and the changelog, and the docstrings and cookbook sentences that described the walk now describe the rule that replaced it. Fixes #20 Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 4 +++ README.md | 2 +- docs/cookbook/02-models.md | 3 +- docs/cookbook/07-slack.md | 9 ++--- grapharc/gateway/config.py | 22 ++++++++---- grapharc/slack/config.py | 11 +++--- tests/test_gateway_openai_ollama.py | 2 +- tests/test_gateway_openrouter.py | 52 ++++++++++++++++++++++++++++- 8 files changed, 85 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb3226f..b51b6a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and "used to be true" — the two things a reader most needs kept apart. Entries are newest-last within a release, matching the order they were written. +## Unreleased + +- the `.env` credential loader **walked up parent directories to `/`**, while the config layer next door refuses exactly that on principle — so the file that *spends money* was discovered more eagerly than the one that *constrains* a run. A run started in a scratch subdirectory picked up an `OPENROUTER_API_KEY` from any ancestor: a `.env` in `$HOME` billed every user's experiment on a shared box to that key, a demo checked out under a client project quietly used the client's key, and since `redact()` is the only thing that ever prints a key, nothing in normal operation said *which file paid*. The rationale `cli/config.py` wrote down for `grapharc.toml` — "a run must never be silently governed by a file in a directory you didn't know about" — applies with more force to the file that pays than to the file that restrains, so `find_env_file` now reads the start directory (default: the working directory) and no ancestor of it. **This is a behaviour change:** anyone relying on a parent-directory `.env` must move it into the directory they run from, `export` the variable, or pass `env_file=` naming the file. Neither escape hatch moved — a real environment variable still beats any file, and an explicit `env_file=` still reads a file anywhere on disk — and no "search boundary" was added in place of the walk, because stopping at a git root is still an upward search. + ## 0.1.3 - `grapharc plan` drives the governed loop; `PolicyEngine.edge_policy()` compiles the TOML document into the gate `AdmissionChecker` consults, and `grapharc plan --policy` is the caller; `grapharc demo --memory PATH` hands the shipped graphs the durable SQLite store. diff --git a/README.md b/README.md index d766092..5f7dbec 100644 --- a/README.md +++ b/README.md @@ -484,7 +484,7 @@ Re-derived on 2026-07-28 by running each item, not by reading the commit log. - **A planning round is an envelope, not a measurement.** A `round` event used to carry the planner's `tokens` and the round's `duration_ms`, both of which `metrics`, `cost` and `replay` add on top of node totals — and the planner's spend was already reported by its own `plan` event, so it was counted twice, and a round's duration encloses the plan plus every node it ran. Neither is on the event now; both are on its `state_delta` as `round_tokens` / `round_iterations` / `round_duration_ms`, where no reader sums them. `RoundRecord.iterations` also holds a figure now rather than always `0`. - **The Claude CLI backend is completion-only, and an agent node on it is *delegated* rather than governed.** The CLI has no tool-calling wire format, so GraphARC cannot run its own gated loop over it. Rather than refuse, `AgentNode` hands the whole loop to Claude Code's headless agent — which means every tool Claude Code has, under its `bypassPermissions` mode: those calls are not checked by this graph's permission policy, not confined by the sandbox executor, and the token figure is the sub-agent's own rather than one GraphARC metered call by call. The workspace boundary and the wall-clock ceiling still hold. It warns on `DelegatedToolUseWarning` at construction and marks every trace event `executor=delegated`, so a run stays auditable as delegated; filter that warning to an error to get the old refusal back. Structured output still needs an OpenAI-wire backend: `openrouter`, `openai`, or a local `ollama`. - **A session turn is synchronous**, and a runner claim is a claim rather than a lease — nothing reclaims a session whose runner died holding it. -- **`.env` is found by walking up parent directories; `grapharc.toml` is not.** The config layer refuses an upward search on purpose — a run must not be governed by a file you did not know about. The credential loader predates that decision and still searches upward, so the thing that *spends money* is discovered more eagerly than the thing that *constrains* it. +- **`.env` and `grapharc.toml` follow the same discovery rule: the working directory, and nowhere else.** Neither searches parent directories — a run must not be governed by a file you did not know about, and must not be *billed* to one either. **This is a behaviour change:** the credential loader used to walk up to `/`, so a `.env` in an ancestor directory (a `$HOME` one on a shared box, a client project one above a demo checkout) was picked up silently. If you relied on that, move the file into the directory you run from, `export` the variable, or pass `env_file=` to name it explicitly. A real environment variable still beats any file. - **`grapharc run` has no budget unless you give it one.** Set any of `--max-tokens`, `--max-iterations`, `--max-seconds`, or `--max-concurrency`; without them each dimension is unlimited and the gate admits a topology of any worst-case cost. **Verified this pass:** `pytest` → 1,754 passed, 12 deselected (the live ones); `ruff check .` clean; all eight `grapharc demo` stages green, plus the `trace` / `metrics` / `viz` / `replay` tour against a freshly recorded demo trace; the wheel builds and imports all 116 submodules in a clean virtualenv with `[all]`. The test count is a snapshot, not a property of the project — `pytest` re-derives it in one command, which is the only reason it is quoted. diff --git a/docs/cookbook/02-models.md b/docs/cookbook/02-models.md index 53a76f1..1c90f75 100644 --- a/docs/cookbook/02-models.md +++ b/docs/cookbook/02-models.md @@ -316,7 +316,8 @@ class Verdict(BaseModel): # The dummy key is only so this snippet runs offline; nothing below opens a # socket. Drop `api_key=` and the backend reads OPENROUTER_API_KEY from the -# environment or the nearest .env. +# environment, or from a .env in the directory you run from — parent +# directories are never searched. model = get_model("openrouter/openai/gpt-4o-mini", api_key="sk-or-not-a-real-key") print(model._llm_type, "|", model.model_name) diff --git a/docs/cookbook/07-slack.md b/docs/cookbook/07-slack.md index 8412280..1e11588 100644 --- a/docs/cookbook/07-slack.md +++ b/docs/cookbook/07-slack.md @@ -114,10 +114,11 @@ Configuration is environment-only, read once at startup: | `GRAPHARC_SLACK_LIVE_INTERVAL` | `2.5` | seconds between two edits of the status message | | `GRAPHARC_SLACK_LIVE_URL` | unset | base URL of a `grapharc serve --live-root` the requester can reach; posts a "watch live" link | -The bot reads tokens from the process environment only. The `.env` -upward-directory search that the model gateway performs is deliberately not -used here: a bot that a whole workspace can drive must not discover -credentials in a file the operator did not point it at. +The bot reads tokens from the process environment only. The model gateway's +`.env` loader is deliberately not used here — even though it now reads the +working directory alone rather than searching upward: a bot that a whole +workspace can drive must not discover credentials in a file the operator did +not point it at, and its working directory is somewhere other things write. ## Live progress diff --git a/grapharc/gateway/config.py b/grapharc/gateway/config.py index 226be25..f60b63d 100644 --- a/grapharc/gateway/config.py +++ b/grapharc/gateway/config.py @@ -5,6 +5,18 @@ `open-router-api-key` cannot be a shell variable at all, so the file has to be parsed rather than sourced. +**The `.env` is read from one directory and no other** — the working directory, +or whatever `start` names — which is the same rule `grapharc.toml` follows, and +for a stronger reason. The config layer refuses an upward search because a run +must never be silently governed by a policy file in a directory the operator did +not know about; this file *spends money*, so a key discovered three directories +up is the worse version of that failure. It used to walk to `/`, which meant a +`.env` in `$HOME` billed every experiment on the box to that key, and `redact` +being the only thing that ever prints a key meant nothing revealed which file +paid. The escape hatches are explicit and unchanged: a real environment variable +still wins over any file, and a caller can still name a file anywhere with +`env_file=`. + Secrets are returned, never logged. Anything that renders a config for humans goes through `redact`. @@ -92,13 +104,9 @@ def _parse_env_file(path: Path) -> dict[str, str]: def find_env_file(start: Path | None = None) -> Path | None: - """Nearest `.env` walking up from `start` (default: cwd).""" - here = (start or Path.cwd()).resolve() - for directory in (here, *here.parents): - candidate = directory / ".env" - if candidate.is_file(): - return candidate - return None + """The `.env` in `start` itself (default: cwd), or None. Parents are not read.""" + candidate = (start or Path.cwd()).resolve() / ".env" + return candidate if candidate.is_file() else None def get_secret(names: tuple[str, ...], *, env_file: Path | None = None) -> str | None: diff --git a/grapharc/slack/config.py b/grapharc/slack/config.py index a2ce492..9b149b9 100644 --- a/grapharc/slack/config.py +++ b/grapharc/slack/config.py @@ -1,11 +1,12 @@ """What the bot needs from its environment, read once at startup. Tokens come from process environment variables only. The gateway's `.env` -loader is deliberately not used here: it searches parent directories upward -(the subject of issue #20), and a bot that anyone in a Slack workspace can -drive must not pick up credentials from a file the operator did not point it -at. `SLACK_BOT_TOKEN` and `SLACK_APP_TOKEN` are exported in the shell that -starts the bot, and nowhere else. +loader is deliberately not used here even now that it reads one directory +rather than walking upward (issue #20): a bot that anyone in a Slack workspace +can drive must not pick up credentials from a file the operator did not point +it at, and the bot's working directory is somewhere other things write. +`SLACK_BOT_TOKEN` and `SLACK_APP_TOKEN` are exported in the shell that starts +the bot, and nowhere else. """ from __future__ import annotations diff --git a/tests/test_gateway_openai_ollama.py b/tests/test_gateway_openai_ollama.py index 4f8d62c..823dc96 100644 --- a/tests/test_gateway_openai_ollama.py +++ b/tests/test_gateway_openai_ollama.py @@ -31,7 +31,7 @@ @pytest.fixture def no_credentials(monkeypatch, tmp_path): - """No key in the environment and no .env anywhere up the tree.""" + """No key in the environment, and a working directory holding no .env.""" for name in (*OPENAI_ENV, *OLLAMA_ENV): monkeypatch.delenv(name, raising=False) monkeypatch.chdir(tmp_path) diff --git a/tests/test_gateway_openrouter.py b/tests/test_gateway_openrouter.py index b5ba283..29d7e89 100644 --- a/tests/test_gateway_openrouter.py +++ b/tests/test_gateway_openrouter.py @@ -53,6 +53,56 @@ def test_process_env_beats_the_file(tmp_path, monkeypatch): assert config.openrouter_api_key(env_file=env) == "from-env" +def test_a_parent_directory_dotenv_is_never_read(tmp_path, monkeypatch): + """The rule `grapharc.toml` follows, on the file that spends money. + + The loader used to walk to `/`, so a run started three directories below a + `.env` — a scratch subdirectory under a client checkout, a `$HOME` one on a + shared box — silently billed against a key the operator never put in scope. + Nothing prints which file paid, so there was no way to notice. The start + directory is now the only directory consulted. + """ + for name in config.OPENROUTER_KEYS: + monkeypatch.delenv(name, raising=False) + (tmp_path / ".env").write_text("OPENROUTER_API_KEY=sk-or-parent\n", encoding="utf-8") + deep = tmp_path / "deeply" / "nested" / "project" + deep.mkdir(parents=True) + + assert config.find_env_file(deep) is None + monkeypatch.chdir(deep) + assert config.find_env_file() is None + assert config.openrouter_api_key() is None + + # ... and the one directory that *is* consulted still is. + (deep / ".env").write_text("OPENROUTER_API_KEY=sk-or-here\n", encoding="utf-8") + assert config.find_env_file() == (deep / ".env").resolve() + assert config.openrouter_api_key() == "sk-or-here" + + +def test_an_explicit_env_file_is_read_wherever_it_lives(tmp_path, monkeypatch): + """The escape hatch for a file outside the working directory: name it.""" + for name in config.OPENROUTER_KEYS: + monkeypatch.delenv(name, raising=False) + elsewhere = tmp_path / "secrets" + elsewhere.mkdir() + (elsewhere / ".env").write_text("OPENROUTER_API_KEY=sk-or-named\n", encoding="utf-8") + run_from = tmp_path / "project" + run_from.mkdir() + monkeypatch.chdir(run_from) + + assert config.openrouter_api_key(env_file=elsewhere / ".env") == "sk-or-named" + + +def test_a_process_variable_beats_a_dotenv_in_the_working_directory(tmp_path, monkeypatch): + """Narrowing discovery did not reorder precedence: the environment wins.""" + (tmp_path / ".env").write_text("OPENROUTER_API_KEY=sk-or-from-file\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + monkeypatch.setenv("OPENROUTER_API_KEY", "sk-or-from-env") + + assert config.find_env_file() == (tmp_path / ".env").resolve() + assert config.openrouter_api_key() == "sk-or-from-env" + + def test_missing_key_returns_none_not_a_crash(tmp_path, monkeypatch): for name in config.OPENROUTER_KEYS: monkeypatch.delenv(name, raising=False) @@ -72,7 +122,7 @@ def test_redact_never_leaks_a_usable_key(secret): def test_constructing_without_a_key_explains_how_to_fix_it(tmp_path, monkeypatch): for name in config.OPENROUTER_KEYS: monkeypatch.delenv(name, raising=False) - monkeypatch.chdir(tmp_path) # no .env anywhere up the tree + monkeypatch.chdir(tmp_path) # an empty directory, so no .env to find with pytest.raises(OpenRouterError, match="OPENROUTER_API_KEY"): OpenRouterChatModel("openai/gpt-4o-mini")