From 2284a1582fb39186b7454cae15b761da2a6b4e27 Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:37:25 -0700 Subject: [PATCH 1/3] fix(cli): load .env when a command runs, not at import (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit codeframe/cli/app.py called load_env_files() at module import, so `from codeframe.cli.app import app` — most of the CLI test suite — wrote the repository's .env into os.environ for the rest of the process. The issue's own reproduction now prints False: $ env -u ANTHROPIC_API_KEY .venv/bin/python -c " import os; from codeframe.cli.app import app print('after cli app import:', bool(os.environ.get('ANTHROPIC_API_KEY')))" after cli app import: False The load moved into the existing Typer root callback, which runs before any command, so `cf` behaviour and the #904 precedence rules are unchanged — they apply a moment later. --help and --version are eager and now short-circuit before it, which is why the "Ignoring security-sensitive key(s)" banner no longer appears on `cf --help`; neither needs credentials. validators.py, the other caller, already calls it inside functions (AC4). Second bug, found by not accepting the new skips at face value: The fix made six tests start skipping, and the reason turned out to be a substring guard in tests/lifecycle/conftest.py: if "lifecycle" in str(item.fspath): That matches test_api_key_lifecycle_919.py, test_proof_requirement_lifecycle_923.py and test_lifecycle_gates_948.py — none of which make an LLM call. So 51 tests have been silently skipped in CI for having "lifecycle" in their filename. It went unnoticed *because* of #1064: the import-time load meant the key was always set locally, so the guard only ever fired in CI, where nobody reads the skip list. Now keyed on the `lifecycle` marker those tests already carry. Verified both directions: the 51 run without a key, and the real lifecycle tests still skip without one and still collect with one. Tests run in child processes, as the AC asks — this module has already imported the CLI transitively, so an in-process assertion would prove nothing. Confirmed non-tautological: restoring the import-time call fails four of six. Full suite: 6407 passed, 49 skipped — same skip set as before the change, +6 passing. --- codeframe/cli/app.py | 11 +- .../cli/test_env_not_loaded_at_import_1064.py | 139 ++++++++++++++++++ tests/lifecycle/conftest.py | 9 +- 3 files changed, 157 insertions(+), 2 deletions(-) create mode 100644 tests/cli/test_env_not_loaded_at_import_1064.py diff --git a/codeframe/cli/app.py b/codeframe/cli/app.py index b3d87d2d..bd554429 100644 --- a/codeframe/cli/app.py +++ b/codeframe/cli/app.py @@ -39,7 +39,11 @@ # environment, and never supplies security-steering keys at all. from codeframe.core.env_provenance import load_env_files # noqa: E402 -load_env_files() +# NOT called here. Loading at import meant `from codeframe.cli.app import app` +# wrote the repository's .env into os.environ for the rest of the process — +# including in tests that never run a command, which silently flipped +# `requires_api_key`-gated tests from skip to run (#1064). The root callback +# below loads it when a command actually executes. # Create main app app = typer.Typer( @@ -292,6 +296,11 @@ def _root( ), ) -> None: """CodeFRAME: Autonomous coding agent orchestration (v2 CLI).""" + # Import-time was too early (#1064): merely importing this module must not + # mutate the ambient environment. Typer runs this before any command, so + # every `cf ...` invocation still sees the operator's .env — the #904 + # precedence rules are unchanged, they just apply a moment later. + load_env_files() # ============================================================================= diff --git a/tests/cli/test_env_not_loaded_at_import_1064.py b/tests/cli/test_env_not_loaded_at_import_1064.py new file mode 100644 index 00000000..6e14991f --- /dev/null +++ b/tests/cli/test_env_not_loaded_at_import_1064.py @@ -0,0 +1,139 @@ +"""#1064 — importing the CLI wrote the repository's .env into os.environ. + +`codeframe/cli/app.py` called `load_env_files()` at module import, so any test +doing `from codeframe.cli.app import app` — most of the CLI suite — loaded the +repo's `.env` into the ambient environment for the rest of the session. That +silently flipped `requires_api_key`-gated tests from skip to run, and forced any +test asserting on an *absent* key to defend itself with `monkeypatch.delenv`. + +The behaviour is correct for `cf` and wrong for `import`. It now happens in the +Typer root callback, which runs before any command. + +Spun off from #946, whose AC3 is "running `pytest tests/` leaves os.environ +unchanged"; #946 fixed the conftest mechanism and named this one as remaining. +""" + +import os +import subprocess +import sys +from pathlib import Path + +import pytest + +pytestmark = pytest.mark.v2 + +REPO_ROOT = Path(__file__).resolve().parents[2] + + +def _child(code: str, *, drop_key: bool = True) -> subprocess.CompletedProcess: + """Run `code` in a fresh interpreter. + + A child process on purpose (the AC asks for it): this test module has almost + certainly already imported the CLI transitively, so asserting in-process + would prove nothing — the environment would already be poisoned. + """ + env = dict(os.environ) + if drop_key: + env.pop("ANTHROPIC_API_KEY", None) + return subprocess.run( + [sys.executable, "-c", code], + cwd=REPO_ROOT, + capture_output=True, + text=True, + env=env, + ) + + +class TestImportingTheCliIsInert: + def test_import_does_not_set_the_api_key(self): + """AC: import with the key unset leaves it unset.""" + result = _child( + "import os\n" + "from codeframe.cli.app import app\n" + "print(bool(os.environ.get('ANTHROPIC_API_KEY')))\n" + ) + assert result.returncode == 0, result.stderr + assert result.stdout.strip().endswith("False"), result.stdout + result.stderr + + def test_import_does_not_print_the_env_loader_notice(self): + """The loader announces refused keys; silence proves it did not run.""" + result = _child( + "from codeframe.cli.app import app\n" + ) + assert result.returncode == 0, result.stderr + combined = result.stdout + result.stderr + assert "security-sensitive" not in combined, combined + + def test_importing_validators_is_inert_too(self): + """AC: the other load_env_files caller is checked for the same pattern. + + validators.py calls it inside functions, not at import — this pins that. + """ + result = _child( + "import os\n" + "import codeframe.cli.validators # noqa: F401\n" + "print(bool(os.environ.get('ANTHROPIC_API_KEY')))\n" + ) + assert result.returncode == 0, result.stderr + assert result.stdout.strip().endswith("False"), result.stdout + result.stderr + + +class TestRunningACommandStillLoadsIt: + """AC: `cf` from a directory with a .env still picks the values up.""" + + def test_a_command_loads_the_env(self): + result = _child( + "import os\n" + "from typer.testing import CliRunner\n" + "from codeframe.cli.app import app\n" + "before = bool(os.environ.get('ANTHROPIC_API_KEY'))\n" + "CliRunner().invoke(app, ['config', 'telemetry', 'status'])\n" + "after = bool(os.environ.get('ANTHROPIC_API_KEY'))\n" + "print(f'{before}->{after}')\n" + ) + assert result.returncode == 0, result.stderr + assert "False->True" in result.stdout, result.stdout + result.stderr + + def test_the_operator_environment_still_wins(self): + """#904 precedence is unchanged — it just applies a moment later.""" + env = dict(os.environ) + env["ANTHROPIC_API_KEY"] = "operator-value" + result = subprocess.run( + [ + sys.executable, "-c", + "import os\n" + "from typer.testing import CliRunner\n" + "from codeframe.cli.app import app\n" + "CliRunner().invoke(app, ['config', 'telemetry', 'status'])\n" + "print(os.environ['ANTHROPIC_API_KEY'])\n", + ], + cwd=REPO_ROOT, + capture_output=True, + text=True, + env=env, + ) + assert result.returncode == 0, result.stderr + assert "operator-value" in result.stdout, result.stdout + result.stderr + + +class TestTheModuleHasNoImportTimeCall: + """A structural check, so the call cannot drift back to module scope.""" + + def test_load_env_files_is_not_called_at_module_level(self): + import ast + + source = (REPO_ROOT / "codeframe" / "cli" / "app.py").read_text() + tree = ast.parse(source) + + offenders = [ + node.lineno + for node in tree.body # module level only + if isinstance(node, ast.Expr) + and isinstance(node.value, ast.Call) + and isinstance(node.value.func, ast.Name) + and node.value.func.id == "load_env_files" + ] + assert offenders == [], ( + f"load_env_files() is called at module scope (app.py:{offenders}); " + "importing the CLI must not mutate os.environ (#1064)" + ) diff --git a/tests/lifecycle/conftest.py b/tests/lifecycle/conftest.py index fb80dce8..e411ea4e 100644 --- a/tests/lifecycle/conftest.py +++ b/tests/lifecycle/conftest.py @@ -55,7 +55,14 @@ def pytest_collection_modifyitems(config, items): if not os.getenv("ANTHROPIC_API_KEY"): skip = pytest.mark.skip(reason="ANTHROPIC_API_KEY not set — lifecycle tests require real API") for item in items: - if "lifecycle" in str(item.fspath): + # The `lifecycle` marker, not a substring of the path. `"lifecycle" + # in str(item.fspath)` also matched test_api_key_lifecycle_919.py, + # test_proof_requirement_lifecycle_923.py and + # test_lifecycle_gates_948.py — none of which make an LLM call — so + # six real tests were being skipped for having "lifecycle" in their + # filename. It went unnoticed because #1064's import-time env load + # meant the key was always set locally; only CI ever skipped them. + if item.get_closest_marker("lifecycle"): item.add_marker(skip) From 0c501d078dca13e7978baa12e5cf206dc456d44e Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Mon, 10 Aug 2026 14:55:28 -0700 Subject: [PATCH 2/3] test(cli): make the env-loading tests hermetic (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI caught this and the reviewer diagnosed it identically: my test asserted "False->True" by depending on the repository's untracked .env to supply the key. So it measured the checkout rather than the behaviour, and passed locally for precisely the reason #1064 exists — the very thing this PR fixes. The tests now build their own .env in a tmp dir and run the child there with PYTHONPATH pointing at the repo, so the file load_env_files finds is one the test created. Verified both with the repo's .env present and with it moved aside, which is CI's condition. The assertions got stronger as a side effect: the load test checks the value is "from-dot-env" rather than merely truthy, and the precedence test asserts the operator's value wins AND that the .env value is absent. Full suite: 6407 passed, 49 skipped. --- .../cli/test_env_not_loaded_at_import_1064.py | 70 ++++++++++++------- 1 file changed, 46 insertions(+), 24 deletions(-) diff --git a/tests/cli/test_env_not_loaded_at_import_1064.py b/tests/cli/test_env_not_loaded_at_import_1064.py index 6e14991f..95ec61f3 100644 --- a/tests/cli/test_env_not_loaded_at_import_1064.py +++ b/tests/cli/test_env_not_loaded_at_import_1064.py @@ -79,41 +79,63 @@ def test_importing_validators_is_inert_too(self): class TestRunningACommandStillLoadsIt: - """AC: `cf` from a directory with a .env still picks the values up.""" + """AC: `cf` from a directory with a .env still picks the values up. - def test_a_command_loads_the_env(self): - result = _child( + These build their own .env in a tmp dir and run the child there. The first + version asserted against the repository's own untracked .env, which passes + on a dev machine and fails in CI, where no such file exists — the test was + measuring the checkout, not the behaviour. + """ + + def _child_in(self, cwd: Path, code: str, env_extra=None): + env = dict(os.environ) + env.pop("ANTHROPIC_API_KEY", None) + env.update(env_extra or {}) + # PYTHONPATH so the child imports codeframe from the repo, while its + # cwd — and therefore the .env load_env_files finds — is the tmp dir. + env["PYTHONPATH"] = str(REPO_ROOT) + os.pathsep + env.get("PYTHONPATH", "") + return subprocess.run( + [sys.executable, "-c", code], + cwd=cwd, + capture_output=True, + text=True, + env=env, + ) + + def test_a_command_loads_the_env(self, tmp_path): + (tmp_path / ".env").write_text("ANTHROPIC_API_KEY=from-dot-env\n") + + result = self._child_in( + tmp_path, "import os\n" "from typer.testing import CliRunner\n" "from codeframe.cli.app import app\n" - "before = bool(os.environ.get('ANTHROPIC_API_KEY'))\n" + "before = os.environ.get('ANTHROPIC_API_KEY')\n" "CliRunner().invoke(app, ['config', 'telemetry', 'status'])\n" - "after = bool(os.environ.get('ANTHROPIC_API_KEY'))\n" - "print(f'{before}->{after}')\n" + "after = os.environ.get('ANTHROPIC_API_KEY')\n" + "print(f'{before}->{after}')\n", ) + assert result.returncode == 0, result.stderr - assert "False->True" in result.stdout, result.stdout + result.stderr + assert "None->from-dot-env" in result.stdout, result.stdout + result.stderr - def test_the_operator_environment_still_wins(self): + def test_the_operator_environment_still_wins(self, tmp_path): """#904 precedence is unchanged — it just applies a moment later.""" - env = dict(os.environ) - env["ANTHROPIC_API_KEY"] = "operator-value" - result = subprocess.run( - [ - sys.executable, "-c", - "import os\n" - "from typer.testing import CliRunner\n" - "from codeframe.cli.app import app\n" - "CliRunner().invoke(app, ['config', 'telemetry', 'status'])\n" - "print(os.environ['ANTHROPIC_API_KEY'])\n", - ], - cwd=REPO_ROOT, - capture_output=True, - text=True, - env=env, + (tmp_path / ".env").write_text("ANTHROPIC_API_KEY=from-dot-env\n") + + result = self._child_in( + tmp_path, + "import os\n" + "from typer.testing import CliRunner\n" + "from codeframe.cli.app import app\n" + "CliRunner().invoke(app, ['config', 'telemetry', 'status'])\n" + "print(os.environ['ANTHROPIC_API_KEY'])\n", + env_extra={"ANTHROPIC_API_KEY": "from-operator"}, ) + assert result.returncode == 0, result.stderr - assert "operator-value" in result.stdout, result.stdout + result.stderr + assert "from-operator" in result.stdout, result.stdout + result.stderr + assert "from-dot-env" not in result.stdout class TestTheModuleHasNoImportTimeCall: From 09aa7e4dff81d3d464c3e6f885a5e0746417dc65 Mon Sep 17 00:00:00 2001 From: Frank Bria <136862992+frankbria@users.noreply.github.com> Date: Mon, 10 Aug 2026 15:12:56 -0700 Subject: [PATCH 3/3] test(cli): isolate HOME too, not just cwd (#1064) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third round on this test. load_env_files reads ~/.env FIRST and both loads use override=False, so on a contributor machine whose real ~/.env carries ANTHROPIC_API_KEY the home value wins and the tmp .env cannot override it — the test fails there and nowhere else. I had isolated cwd and left HOME alone. _child_in now points HOME (and USERPROFILE, for Path.home() on Windows) at an empty directory beside the fixture. Verified by constructing that machine rather than reasoning about it: a fake $HOME containing ANTHROPIC_API_KEY=sk-from-home-env reproduces the predicted failure without the isolation and passes with it. Green across all four combinations of home-.env and repo-.env presence. Full suite: 6407 passed, 49 skipped. --- tests/cli/test_env_not_loaded_at_import_1064.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/cli/test_env_not_loaded_at_import_1064.py b/tests/cli/test_env_not_loaded_at_import_1064.py index 95ec61f3..d5b15054 100644 --- a/tests/cli/test_env_not_loaded_at_import_1064.py +++ b/tests/cli/test_env_not_loaded_at_import_1064.py @@ -90,6 +90,14 @@ class TestRunningACommandStillLoadsIt: def _child_in(self, cwd: Path, code: str, env_extra=None): env = dict(os.environ) env.pop("ANTHROPIC_API_KEY", None) + # HOME too, not just cwd: load_env_files reads ~/.env FIRST and both + # loads use override=False, so a contributor whose real ~/.env carries + # ANTHROPIC_API_KEY would win over the tmp .env and this test would + # fail on their machine. Point HOME at an empty dir beside the fixture. + empty_home = cwd / "home" + empty_home.mkdir(exist_ok=True) + env["HOME"] = str(empty_home) + env["USERPROFILE"] = str(empty_home) # Path.home() on Windows env.update(env_extra or {}) # PYTHONPATH so the child imports codeframe from the repo, while its # cwd — and therefore the .env load_env_files finds — is the tmp dir.