diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e4a429..0b15fcf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ Entries are newest-last within a release, matching the order they were written. ## Unreleased - a run **stopped for overspending reported spending nothing**. Tokens were attributed from `end` events, and a node the budget interrupts emits `error` instead — so `grapharc metrics` answered `tokens: 0` for a run whose own enforcement message named the figure that stopped it (`max_tokens reached (51/5)`). The audit trail lost precisely the number the stop was about, and per-node attribution dropped the most expensive node in the run. Every `error` event is now stamped with what its node spent, exactly as `end` is, and both `summarize` and the cost report count it; sub-events inside a node remain a breakdown of its total rather than an addition, so the disjointness that kept `ends + orphans` from double-counting is unchanged, and `RunCost.tokens == RunMetrics.tokens` still holds. +- 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 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")