From b26ccaf440e67eea04765ad10fe0c346fcbbaff2 Mon Sep 17 00:00:00 2001 From: Joao Moura Date: Sun, 18 Oct 2026 23:31:53 -0700 Subject: [PATCH] Never run the crew toward an AMP the login must not reach; check before asking `crewai eval` now refuses to run the crew whenever a credential of this machine would go with the trace to an AMP it does not trust, whatever the tracing setting: `tracing=True` in code is invisible to the CLI, and a project's crewai older than the handshake sends the credential wherever the run traces. The run app's Evaluate makes the same check before it asks anything, so a yes can never write CREWAI_TRACING_ENABLED=true to `.env` for a re-run that is then refused. Which credentials are this machine's is read from the environment as it was before any project `.env` was loaded (`run_crew.environment_before_project`), so on Evaluate's re-run a CREWAI_USER_PAT the project's own `.env` brought is the project's, as on any run of it. Co-Authored-By: Claude Opus 5.5 --- lib/cli/src/crewai_cli/crew_run_tui.py | 14 +++ .../src/crewai_cli/experimental/eval_crew.py | 45 +++++--- lib/cli/src/crewai_cli/run_crew.py | 20 +++- lib/cli/tests/conftest.py | 10 ++ lib/cli/tests/experimental/test_eval_crew.py | 106 ++++++++++++++---- lib/cli/tests/test_crew_run_tui.py | 45 ++++++++ 6 files changed, 200 insertions(+), 40 deletions(-) diff --git a/lib/cli/src/crewai_cli/crew_run_tui.py b/lib/cli/src/crewai_cli/crew_run_tui.py index 20b8ec98b..bb8727b13 100644 --- a/lib/cli/src/crewai_cli/crew_run_tui.py +++ b/lib/cli/src/crewai_cli/crew_run_tui.py @@ -1457,8 +1457,12 @@ FooterKey .footer-key--key { # A project that turned it off, or never turned it on, is asked # first; a no runs nothing. from crewai_cli.experimental.eval_crew import ( + _machine_amp_origins, + _machine_credential, + record_stop, tracing_already_on, tracing_turned_off, + untrusted_amp_stop, ) if self._tracing_off_in_code(): @@ -1469,6 +1473,16 @@ FooterKey .footer-key--key { severity="warning", ) return + # Before anything is asked or written: a run whose trace would carry + # this machine's login to an AMP it does not trust is not run again + # (`crewai eval` refuses it too), so a yes here could not be honored. + # This process loaded the project's `.env` for the run, so only the + # AMPs this machine is configured for are trusted. + stop = untrusted_amp_stop(_machine_amp_origins(), _machine_credential()) + if stop is not None: + record_stop("untrusted_amp") + self.notify(stop, title="Evaluate", severity="warning") + return if tracing_turned_off(): self.push_screen(TurnTracingOnScreen(), self._tracing_on_answered) return diff --git a/lib/cli/src/crewai_cli/experimental/eval_crew.py b/lib/cli/src/crewai_cli/experimental/eval_crew.py index 32f1436ac..200b685b6 100644 --- a/lib/cli/src/crewai_cli/experimental/eval_crew.py +++ b/lib/cli/src/crewai_cli/experimental/eval_crew.py @@ -195,9 +195,9 @@ NOT_SHARED = ( ) UNTRUSTED_AMP = ( "The crew was not run: its trace would go to {url}, an AMP this machine does not " - "send its CrewAI login to, and with tracing on this project's crewai would send the " - "login there. Log in to that AMP (`crewai enterprise configure {url}`) or remove " - "CREWAI_PLUS_URL from the project's .env, then run `crewai eval` again." + "send its CrewAI login to, and the project's crewai could send the login there " + "with the trace. Log in to that AMP (`crewai enterprise configure {url}`) or " + "remove CREWAI_PLUS_URL from the project's .env, then evaluate again." ) @@ -779,8 +779,11 @@ def _project_changed_since(record: dict[str, Any]) -> bool: def _machine_credential() -> bool: """Does this machine hold a credential a run's tracing would send — the saved `crewai login`, or a PAT or integration token in the environment - before the project's `.env` is loaded?""" - if os.environ.get("CREWAI_USER_PAT") or os.environ.get( + before the project's `.env` is loaded? A token the `.env` brought is the + project's own, read here even after the run app loaded it.""" + from crewai_cli.run_crew import environment_before_project + + if environment_before_project("CREWAI_USER_PAT") or environment_before_project( "CREWAI_ENTERPRISE_ACTION_AUTH_TOKEN" ): return True @@ -790,6 +793,15 @@ def _machine_credential() -> bool: return False # unreadable here is unreadable to the run too +def untrusted_amp_stop(trusted: set[str], machine_credential: bool) -> str | None: + """Why the crew must not run for an evaluation, when a credential of this + machine would go with its trace to an AMP this command does not trust; + None when it may run.""" + if not _credential_would_leave(trusted, machine_credential): + return None + return UNTRUSTED_AMP.format(url=PlusAPI().base_url.rstrip("/")) + + def _credential_would_leave(trusted: set[str], machine_credential: bool) -> bool: """Would a credential of this machine go to an AMP this command does not trust — the one the run traces to, read after the project's `.env`?""" @@ -934,8 +946,9 @@ def _run_the_crew_to_evaluate( ) -> str | None: """Run the crew, traced, to have a run to evaluate. - No question first, with or without a terminal: running `crewai eval` asked - for the run to be graded, and the grade is read off its trace. The run app + Running `crewai eval` asked for the run to be graded, and the grade is read + off its trace; the only question is the first enable of tracing, put to a + person at a terminal (`_ask_before_the_first_enable`). The run app evaluates what it ran, on its own screen, so this returns None when it did — the reader has already seen the link and the verdict. It returns the run to grade when no app got to it, and says so itself when nothing was traced. @@ -950,16 +963,14 @@ def _run_the_crew_to_evaluate( ) # A credential of this machine must never reach an AMP this command does # not trust, and a project's crewai older than the handshake sends it to - # whatever AMP the project names once tracing is on — the CLI cannot tell - # which crewai the project runs. So with tracing on for this run (the - # project's .env, a consent given once) the crew is not run at all, and a - # project that turned tracing off is not asked to turn it on toward such an - # AMP. With tracing unset it is not turned on, and a crewai that knows the - # handshake still traces the run, anonymously, there. - leaves = _credential_would_leave(trusted or set(), machine_credential) - if leaves and (tracing_already_on() or tracing_turned_off()): + # whatever AMP the project names whenever the run traces — by the project's + # .env, a consent given once, or `tracing=True` in code, which the CLI + # cannot see; nor can it tell which crewai the project runs. So the crew + # is not run at all, and nobody is asked to turn tracing on toward it. + stop = untrusted_amp_stop(trusted or set(), machine_credential) + if stop is not None: record_stop("untrusted_amp") - _fail(UNTRUSTED_AMP.format(url=PlusAPI().base_url.rstrip("/"))) + _fail(stop) if tracing_turned_off(): _ask_to_turn_tracing_on() # Nobody turned tracing on. At a terminal the person is asked first, on @@ -968,7 +979,7 @@ def _run_the_crew_to_evaluate( # for the project — said once, plainly, here and in the run app. From # then on the project traces, and nothing more is asked or said. turned_on_now = False - if not leaves and not tracing_already_on(): + if not tracing_already_on(): if _at_a_terminal(): _ask_before_the_first_enable() else: diff --git a/lib/cli/src/crewai_cli/run_crew.py b/lib/cli/src/crewai_cli/run_crew.py index af825ecda..89854ad0d 100644 --- a/lib/cli/src/crewai_cli/run_crew.py +++ b/lib/cli/src/crewai_cli/run_crew.py @@ -532,13 +532,29 @@ def _record_deploy_button(event: DeployButtonEvent) -> None: EVAL_TOKEN_ENV = "CREWAI_EVAL_AWAITING_RUN" # noqa: S105 - a variable name, not a secret +# What each project `.env` loaded into this process replaced: the value the +# process had before (None: unset). The first load is the one kept. +_ENV_BEFORE_PROJECT: dict[str, str | None] = {} + + +def environment_before_project(key: str) -> str | None: + """KEY as this process had it before any project's `.env` was loaded over + it: what belongs to this machine, not to the project.""" + if key in _ENV_BEFORE_PROJECT: + return _ENV_BEFORE_PROJECT[key] + return os.environ.get(key) + + def load_project_env(env_file: Path) -> None: """The project's `.env` over this process's environment, `crewai eval`'s token excepted: the token this process was started with is kept, and a `.env` never sets one. Otherwise a `.env` could unmark a run `crewai eval` - started, and its tracing would no longer apply the command's rules.""" - from dotenv import load_dotenv + started, and its tracing would no longer apply the command's rules. What + it replaced stays readable (`environment_before_project`).""" + from dotenv import dotenv_values, load_dotenv + for key in dotenv_values(env_file): + _ENV_BEFORE_PROJECT.setdefault(key, os.environ.get(key)) kept = os.environ.get(EVAL_TOKEN_ENV) load_dotenv(env_file, override=True) if kept is None: diff --git a/lib/cli/tests/conftest.py b/lib/cli/tests/conftest.py index 420fbdbb3..7471b2ca3 100644 --- a/lib/cli/tests/conftest.py +++ b/lib/cli/tests/conftest.py @@ -11,3 +11,13 @@ def _usage_counts_stay_on_this_machine(monkeypatch): A test that checks the counting replaces `Telemetry` or sets the flag itself.""" monkeypatch.setenv("CREWAI_DISABLE_TELEMETRY", "true") + + +@pytest.fixture(autouse=True) +def _each_test_starts_before_any_project_env(monkeypatch): + """`load_project_env` remembers what a project's `.env` replaced, for the + life of the process; a test's `.env` must not become the next test's + "before".""" + from crewai_cli import run_crew + + monkeypatch.setattr(run_crew, "_ENV_BEFORE_PROJECT", {}) diff --git a/lib/cli/tests/experimental/test_eval_crew.py b/lib/cli/tests/experimental/test_eval_crew.py index ade4cf9ba..c24c266c7 100644 --- a/lib/cli/tests/experimental/test_eval_crew.py +++ b/lib/cli/tests/experimental/test_eval_crew.py @@ -3300,15 +3300,19 @@ def test_this_commands_own_project_id_write_is_no_change(project, monkeypatch): # ── the saved login and an AMP the project chose ───────────────────────────── -def test_a_rerun_from_the_run_app_trusts_only_what_the_app_says(project, monkeypatch): +@pytest.mark.parametrize("project_amp", ["https://amp.test", "https://other.example"]) +def test_a_rerun_from_the_run_app_trusts_only_what_the_app_says( + project, monkeypatch, usage, project_amp +): """The run app's process loaded the project's `.env` already, so its CREWAI_PLUS_URL cannot vouch for itself: the app passes this machine's - AMPs, and neither the evaluation nor the run trusts anything else.""" + AMPs, the run is told only those, and a run whose trace would carry this + machine's login anywhere else is not run.""" from crewai_core.eval_handshake import awaited_evaluation directory, _ = project (directory / "pyproject.toml").write_text("[project]\nname = 'demo'\n") - monkeypatch.setenv("CREWAI_PLUS_URL", "https://other.example") # from the project's .env + monkeypatch.setenv("CREWAI_PLUS_URL", project_amp) # from the project's .env told: list[object] = [] def fake_run_crew() -> None: @@ -3318,10 +3322,60 @@ def test_a_rerun_from_the_run_app_trusts_only_what_the_app_says(project, monkeyp monkeypatch.setattr(run_crew_module, "run_crew", fake_run_crew) amp = install(monkeypatch, FakeAMP(statuses=[done()])) - eval_module.eval_crew(run_again=True, trusted_amp_origins={"https://amp.test"}) + if project_amp == "https://amp.test": + eval_module.eval_crew(run_again=True, trusted_amp_origins={"https://amp.test"}) + assert told == [["https://amp.test"]] + else: + with pytest.raises(SystemExit) as exited: + eval_module.eval_crew(run_again=True, trusted_amp_origins={"https://amp.test"}) + assert exited.value.code == 1 + assert told == [] and amp.calls == [] + assert usage == ["cli_usage:eval_stopped:untrusted_amp"] - assert told == [["https://amp.test"]] - assert amp.api_key is None # read anonymously: other.example gets no login + +def _put_back_after(monkeypatch, *keys: str) -> None: + """`load_project_env` writes `os.environ` itself: the test puts it back.""" + for key in keys: + monkeypatch.setenv(key, "unset-after-the-test") + monkeypatch.delenv(key) + + +@pytest.mark.parametrize("machine_pat", [None, "the-machines"]) +def test_a_token_the_projects_env_brought_is_the_projects_own( + project, monkeypatch, usage, machine_pat +): + """Evaluate's re-run happens in the process that loaded the project's `.env` + for the run the person watched. A CREWAI_USER_PAT that `.env` brought goes + where the same `.env` says on every run of the project, so the crew runs; a + token this machine had before the `.env` replaced it is still this + machine's, and does not go to an AMP it does not trust.""" + directory, _ = project + (directory / "pyproject.toml").write_text("[project]\nname = 'demo'\n") + (directory / ".env").write_text( + "CREWAI_PLUS_URL=https://self-hosted.example\nCREWAI_USER_PAT=the-projects\n" + ) + _put_back_after(monkeypatch, "CREWAI_PLUS_URL", "CREWAI_USER_PAT", "CREWAI_ENTERPRISE_ACTION_AUTH_TOKEN") + monkeypatch.setattr(eval_module, "saved_login", lambda: None) + if machine_pat: + monkeypatch.setenv("CREWAI_USER_PAT", machine_pat) + run_crew_module.load_project_env(directory / ".env") # the run app's own load + ran: list[str] = [] + + def fake_run_crew() -> None: + ran.append(os.environ["CREWAI_USER_PAT"]) + record_last_run(directory, "the-new-run", recorded_at=_now()) + + monkeypatch.setattr(run_crew_module, "run_crew", fake_run_crew) + install(monkeypatch, FakeAMP(statuses=[done()])) + + if machine_pat: + with pytest.raises(SystemExit): + eval_module.eval_crew(run_again=True, trusted_amp_origins=eval_module._machine_amp_origins()) + assert ran == [] + assert usage == ["cli_usage:eval_stopped:untrusted_amp"] + else: + eval_module.eval_crew(run_again=True, trusted_amp_origins=eval_module._machine_amp_origins()) + assert ran == ["the-projects"] @pytest.mark.parametrize("amp_url", ["https://evil.example", "https://amp.test"]) @@ -3329,9 +3383,8 @@ def test_tracing_is_never_turned_on_toward_an_amp_the_command_does_not_trust( project, monkeypatch, capsys, amp_url ): """A project's crewai older than the handshake sends the saved login to the - AMP its `.env` names once tracing is on: toward an untrusted AMP the command - leaves tracing as it was, and a crewai that knows the handshake still traces - the run, anonymously, there.""" + AMP its `.env` names whenever the run traces: toward an untrusted AMP the + command leaves tracing as it was and does not run the crew.""" from crewai_core.eval_handshake import awaited_evaluation directory, _ = project @@ -3345,34 +3398,44 @@ def test_tracing_is_never_turned_on_toward_an_amp_the_command_does_not_trust( record_last_run(directory, "the-new-run", recorded_at=_now()) monkeypatch.setattr(run_crew_module, "run_crew", fake_run_crew) + monkeypatch.setattr(eval_module.sys.stdin, "isatty", lambda: False) install(monkeypatch, FakeAMP(statuses=[done()])) - eval_module.eval_crew() - trusted = amp_url == "https://amp.test" - assert seen == ([("true", True)] if trusted else [(None, False)]) + if trusted: + eval_module.eval_crew() + else: + with pytest.raises(SystemExit): + eval_module.eval_crew() + + assert seen == ([("true", True)] if trusted else []) assert ("CREWAI_TRACING_ENABLED=true" in (directory / ".env").read_text()) is trusted assert ("Tracing is now on" in capsys.readouterr().out) is trusted -@pytest.mark.parametrize("tracing_on_by", ["the project's .env", "a consent given once"]) +@pytest.mark.parametrize("at_a_terminal", [False, True]) +@pytest.mark.parametrize( + "tracing_on_by", + ["the project's .env", "a consent given once", "nobody, or tracing=True in code"], +) def test_a_crew_whose_trace_would_carry_the_login_to_an_untrusted_amp_is_not_run( - project, monkeypatch, capsys, usage, tracing_on_by + project, monkeypatch, capsys, usage, consent_screen, tracing_on_by, at_a_terminal ): - """With tracing already on, a project crewai older than the handshake would - send this machine's login to the AMP its `.env` names; the CLI cannot tell - which crewai the project runs, so the crew is not run and the sentence says - how to go on.""" + """A project crewai older than the handshake would send this machine's + login to the AMP its `.env` names whenever the run traces, `tracing=True` + in code included, which the CLI cannot see; nor can it tell which crewai + the project runs. So the crew is not run, nobody is asked to turn tracing + on, and the sentence says how to go on.""" directory, _ = project (directory / "pyproject.toml").write_text("[project]\nname = 'demo'\n") env = "CREWAI_PLUS_URL=https://evil.example\n" if tracing_on_by == "the project's .env": env += "CREWAI_TRACING_ENABLED=true\n" - else: + elif tracing_on_by == "a consent given once": monkeypatch.setattr("crewai_core.user_data.is_tracing_enabled", lambda: True) (directory / ".env").write_text(env) - monkeypatch.setattr(eval_module.sys.stdin, "isatty", lambda: False) + monkeypatch.setattr(eval_module.sys.stdin, "isatty", lambda: at_a_terminal) amp = install(monkeypatch, FakeAMP()) with pytest.raises(SystemExit) as exited: @@ -3383,7 +3446,8 @@ def test_a_crew_whose_trace_would_carry_the_login_to_an_untrusted_amp_is_not_run assert "The crew was not run: its trace would go to https://evil.example" in out assert "crewai enterprise configure https://evil.example" in out assert usage == ["cli_usage:eval_stopped:untrusted_amp"] - assert amp.calls == [] + assert amp.calls == [] and consent_screen.asked == [] + assert (directory / ".env").read_text() == env # nothing turned on def test_with_no_credential_on_this_machine_there_is_nothing_to_protect(project, monkeypatch): diff --git a/lib/cli/tests/test_crew_run_tui.py b/lib/cli/tests/test_crew_run_tui.py index 803f4a166..bd4f5be48 100644 --- a/lib/cli/tests/test_crew_run_tui.py +++ b/lib/cli/tests/test_crew_run_tui.py @@ -2570,6 +2570,51 @@ def test_evaluate_on_an_untraced_run_asks_before_tracing_is_turned_on( assert stops == ["cli_usage:eval_stopped:declined"] +@pytest.mark.parametrize("tracing", ["turned off", "never turned on", "on"]) +def test_evaluate_toward_an_untrusted_amp_asks_nothing_and_writes_nothing( + monkeypatch, tmp_path, stops, tracing +) -> None: + """The project's `.env` names an AMP this machine does not send its login + to: `crewai eval` would refuse the re-run, so nothing is asked, `.env` is + left as it was, and the app says why instead of closing.""" + from crewai_cli.experimental import eval_crew + + monkeypatch.chdir(tmp_path) + env = "CREWAI_PLUS_URL=https://evil.example\n" + if tracing == "turned off": + env += "CREWAI_TRACING_ENABLED=false\n" + if tracing == "on": + env += "CREWAI_TRACING_ENABLED=true\n" + (tmp_path / ".env").write_text(env) + monkeypatch.setenv("CREWAI_PLUS_URL", "https://evil.example") # the run's own load + monkeypatch.delenv("CREWAI_TRACING_ENABLED", raising=False) + monkeypatch.setattr(eval_crew, "saved_login", lambda: "login-token") + monkeypatch.setattr( + eval_crew, + "Settings", + lambda: SimpleNamespace(enterprise_base_url="https://app.crewai.com"), + ) + app = CrewRunApp() + app._status = "completed" + app._crew_result = object() + app._telemetry = Mock() + app._execution_uuid = "run-this-app" + app._unsubscribe = lambda: None # type: ignore[method-assign] + exits: list[object] = [] + app.exit = lambda result=None: exits.append(result) # type: ignore[method-assign] + app.push_screen = lambda *a, **k: pytest.fail("nothing is asked") # type: ignore[method-assign] + notices: list[str] = [] + app.notify = lambda message, **k: notices.append(message) # type: ignore[method-assign] + monkeypatch.setattr(crew_run_tui, "_trace_was_recorded", lambda uuid: False) + + app.action_evaluate_crew() + + assert exits == [] and app._want_traced_rerun is False + assert (tmp_path / ".env").read_text() == env + assert notices and "its trace would go to https://evil.example" in notices[0] + assert stops == ["cli_usage:eval_stopped:untrusted_amp"] + + def test_pressing_evaluate_on_an_untraced_run_counts_the_press_and_nothing_else( monkeypatch, stops ) -> None: