fix(cli): the saved login goes only to an AMP this machine is logged in to; only a missing login reads as anonymous

Two review findings. - Loading the project's .env (so `crewai eval` asks the AMP the run was traced to) also let a project's .env choose where the saved bearer token goes. The request still follows the project's CREWAI_PLUS_URL, because that is how a self-hosted project is wired and the run really is there, but the credential now goes only to an origin this machine is logged in to: `crewai enterprise configure`'s saved settings, an address already exported in this shell (read before .env is loaded), or app.crewai.com. Anywhere else the run is read anonymously and the command says so, naming `crewai enterprise configure`. The wider hole is not this command's: `crewai run` sends the same token to the same .env-chosen URL, and that is worth a separate look. - saved_login() caught every exception and returned None, so an unreadable credential store — a rotated key, a directory left owned by root — read as "anonymous", quietly spending the run's one anonymous read and then refusing a user who believes they are logged in. Only AuthError means anonymous now; anything else is reported with its cause and a pointer to `crewai login`. This matches tracing_credential() in the library, which catches AuthError alone. Tests: a project pointing elsewhere is read anonymously with the message and no token; a shell-exported AMP is trusted; the configured AMP keeps the token and says nothing; the origin rule itself; AuthError versus an unreadable store. 41 passed. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
2026-10-12 02:12:31 +00:00 · 2026-09-20 22:42:43 -07:00
parent d849f9d7d1
commit 0f6e499df5
2 changed files with 138 additions and 11 deletions
@@ -21,14 +21,17 @@ from pathlib import Path
 import sys
 import time
 from typing import Any
+from urllib.parse import urlparse
 import webbrowser

 import click
+from crewai_core.constants import DEFAULT_CREWAI_ENTERPRISE_URL
+from crewai_core.settings import Settings
 from dotenv import load_dotenv, set_key
 import httpx
 from rich.console import Console

-from crewai_cli.authentication.token import get_auth_token
+from crewai_cli.authentication.token import AuthError, get_auth_token
 from crewai_cli.plus_api import PlusAPI
 from crewai_cli.utils import get_or_create_project_id, is_dmn_mode_enabled

@@ -46,6 +49,8 @@ STATUSES = {"queued", "running"} | FINISHED
 def eval_crew(run_id: str | None = None) -> None:
    """Evaluate the last traced run of this project, or the run RUN_ID."""
    get_or_create_project_id()
+    # Read before the project's .env is loaded, so a project cannot add itself.
+    trusted = _trusted_amp_origins()
    _load_project_env()
    record = read_last_run() or {}
    execution_id = run_id or record.get("execution_id")
@@ -53,9 +58,7 @@ def eval_crew(run_id: str | None = None) -> None:
        execution_id = _run_now_or_explain()
        record = read_last_run() or {}

-    # The credential goes to the CONFIGURED AMP only (CREWAI_PLUS_URL, the saved settings,
-    # app.crewai.com) — never to an address read off a file in the project.
-    client = PlusAPI(api_key=saved_login())
+    client = _amp_client(trusted)
    recorded_amp = str(record.get("amp_base_url") or "").rstrip("/")
    if not run_id and recorded_amp and recorded_amp != client.base_url.rstrip("/"):
        console.print(
@@ -75,6 +78,46 @@ def eval_crew(run_id: str | None = None) -> None:
        raise SystemExit(1)


+def _trusted_amp_origins() -> set[str]:
+    """Where the saved login may be sent: the AMP this machine is configured for
+    (`crewai enterprise configure`), one already exported in this shell, and
+    crewAI's own."""
+    candidates = (
+        os.environ.get("CREWAI_PLUS_URL"),
+        Settings().enterprise_base_url,
+        DEFAULT_CREWAI_ENTERPRISE_URL,
+    )
+    return {origin for origin in map(_origin, candidates) if origin}
+
+
+def _origin(url: str | None) -> str | None:
+    parsed = urlparse(str(url or ""))
+    return (
+        f"{parsed.scheme}://{parsed.netloc}".lower()
+        if parsed.scheme and parsed.netloc
+        else None
+    )
+
+
+def _amp_client(trusted: set[str]) -> PlusAPI:
+    """The AMP to ask, and whether the saved login goes with it.
+
+    A project's `.env` may point `crewai eval` at another AMP — that is how a
+    self-hosted project is wired, and the run was traced there — so the request
+    follows it. The credential does not: it goes only to an AMP this machine is
+    logged in to, and the run is read anonymously anywhere else."""
+    client = PlusAPI(api_key=saved_login())
+    if client.api_key is None or _origin(client.base_url) in trusted:
+        return client
+
+    console.print(
+        f"Reading anonymously: {client.base_url} is not an AMP this machine is logged in to. "
+        "Run `crewai enterprise configure <url>` to log in to it.",
+        style="yellow",
+    )
+    return PlusAPI()
+
+
 def _load_project_env() -> None:
    """The project's .env, as `crewai run` loads it — so CREWAI_PLUS_URL here is the one the run used."""
    env_file = Path.cwd() / ".env"
@@ -97,10 +140,22 @@ def read_last_run(directory: Path | None = None) -> dict[str, Any] | None:


 def saved_login() -> str | None:
-    """The `crewai login` token, or None: AMP then treats the caller as anonymous."""
+    """The `crewai login` token, or None: AMP then treats the caller as anonymous.
+
+    Only "not logged in" reads as anonymous. A credential that exists but cannot
+    be read — a rotated key, a half-written store, a directory `sudo` left owned
+    by root — is said out loud: reading anonymously instead would quietly spend
+    the run's one anonymous read and then refuse a user who believes they are
+    logged in."""
    try:
        return get_auth_token()
-    except Exception:
+    except AuthError:
+        return None
+    except Exception as error:
+        _fail(
+            f"Could not read the saved login ({type(error).__name__}: {error}). "
+            "Run `crewai login` again, or `crewai eval` will not know who you are."
+        )
        return None


@@ -3,7 +3,9 @@
 from __future__ import annotations

 import json
+import os
 from pathlib import Path
+from types import SimpleNamespace

 from click.testing import CliRunner
 import httpx
@@ -53,6 +55,9 @@ def project(tmp_path, monkeypatch):
    monkeypatch.setattr(eval_module, "saved_login", lambda: "login-token")
    monkeypatch.setattr(eval_module.time, "sleep", lambda seconds: None)
    monkeypatch.setattr(eval_module, "is_dmn_mode_enabled", lambda: False)
+    # This machine is logged in to https://amp.test (`crewai enterprise configure`).
+    monkeypatch.setattr(eval_module, "Settings", lambda: SimpleNamespace(enterprise_base_url="https://amp.test"))
+    monkeypatch.delenv("CREWAI_PLUS_URL", raising=False)
    opened: list[str] = []
    monkeypatch.setattr(eval_module.webbrowser, "open", lambda url: opened.append(url) or True)
    return tmp_path, opened
@@ -66,7 +71,9 @@ def record_last_run(directory: Path, execution_id: str = EXECUTION_ID, **fields)

 def install(monkeypatch, amp: FakeAMP, configured_amp: str = "https://amp.test") -> FakeAMP:
    def build(api_key=None, base_url=None):
-        amp.api_key, amp.base_url = api_key, base_url or configured_amp
+        # PlusAPI's own resolution: explicit, then CREWAI_PLUS_URL, then the saved settings.
+        amp.api_key = api_key
+        amp.base_url = base_url or os.environ.get("CREWAI_PLUS_URL") or configured_amp
        return amp

    monkeypatch.setattr(eval_module, "PlusAPI", build)
@@ -151,14 +158,60 @@ def test_the_credential_goes_only_to_the_configured_amp_never_to_an_address_off_
    out = capsys.readouterr().out
    assert "The run was traced to https://evil.example/steal; evaluating at the configured AMP https://app.crewai.com." in out

-    # The project's .env is what `crewai run` traced with, so it is loaded first: same AMP, no note.
+    # The project's .env is what `crewai run` traced with, so it is loaded first. This machine is
+    # logged in to that AMP, so the credential goes with the request and nothing is remarked on.
    (directory / ".env").write_text("CREWAI_PLUS_URL=https://amp.test\n")
    record_last_run(directory, amp_base_url="https://amp.test/")
-    install(monkeypatch, FakeAMP(statuses=[done()]))
-    monkeypatch.delenv("CREWAI_PLUS_URL", raising=False)
+    amp = install(monkeypatch, FakeAMP(statuses=[done()]))
    eval_module.eval_crew()
    assert eval_module.os.environ["CREWAI_PLUS_URL"] == "https://amp.test"
-    assert "was traced to" not in capsys.readouterr().out
+    assert amp.api_key == "login-token" and amp.base_url == "https://amp.test"
+    out = capsys.readouterr().out
+    assert "was traced to" not in out and "Reading anonymously" not in out
+
+
+def test_a_project_may_point_at_another_amp_but_never_gets_the_saved_login(project, monkeypatch, capsys):
+    """A .env can send the request elsewhere — that is how a self-hosted project is wired —
+    but the token goes only to an AMP this machine is logged in to."""
+    directory, _ = project
+    (directory / ".env").write_text("CREWAI_PLUS_URL=https://evil.example\n")
+    record_last_run(directory, amp_base_url="https://evil.example")
+    amp = install(monkeypatch, FakeAMP(statuses=[done()]))
+
+    eval_module.eval_crew()  # still works: AMP reads it anonymously
+
+    assert amp.base_url == "https://evil.example"  # the request follows the project
+    assert amp.api_key is None  # the credential does not
+    out = capsys.readouterr().out
+    assert "Reading anonymously: https://evil.example is not an AMP this machine is logged in to." in out
+    assert "crewai enterprise configure" in out
+
+
+def test_an_amp_exported_in_this_shell_is_trusted(project, monkeypatch, capsys):
+    directory, _ = project
+    monkeypatch.setenv("CREWAI_PLUS_URL", "https://shell.amp.test")  # exported before the project is read
+    record_last_run(directory, amp_base_url="https://shell.amp.test")
+    amp = install(monkeypatch, FakeAMP(statuses=[done()]))
+
+    eval_module.eval_crew()
+
+    assert amp.base_url == "https://shell.amp.test" and amp.api_key == "login-token"
+    assert "Reading anonymously" not in capsys.readouterr().out
+
+
+@pytest.mark.parametrize(
+    ("url", "origin"),
+    [
+        ("https://amp.test", "https://amp.test"),
+        ("https://AMP.Test/crewai_plus/", "https://amp.test"),
+        ("http://localhost:8000/x", "http://localhost:8000"),
+        ("app.crewai.com", None),  # no scheme: not an origin, never trusted
+        ("", None),
+        (None, None),
+    ],
+)
+def test_an_origin_is_scheme_and_host_only(url, origin):
+    assert eval_module._origin(url) == origin


@pytest.mark.parametrize(
@@ -366,6 +419,25 @@ def test_the_cli_command_maps_to_the_implementation(monkeypatch):
    assert "Evaluate the last traced run" in runner.invoke(eval_command, ["--help"]).output


+def test_only_a_missing_login_reads_as_anonymous(monkeypatch, capsys):
+    """An unreadable credential store is not "anonymous": it is said out loud."""
+    from crewai_cli.authentication.token import AuthError
+
+    monkeypatch.setattr(eval_module, "get_auth_token", lambda: (_ for _ in ()).throw(AuthError("No token found")))
+    assert eval_module.saved_login() is None  # not logged in: AMP treats the caller as anonymous
+
+    monkeypatch.setattr(eval_module, "get_auth_token", lambda: "login-token")
+    assert eval_module.saved_login() == "login-token"
+
+    for broken in (OSError(13, "Permission denied"), ValueError("Fernet key must be 32 url-safe base64-encoded bytes.")):
+        monkeypatch.setattr(eval_module, "get_auth_token", lambda error=broken: (_ for _ in ()).throw(error))
+        with pytest.raises(SystemExit) as exit_:
+            eval_module.saved_login()
+        assert exit_.value.code == 1
+        out = capsys.readouterr().out
+        assert "Could not read the saved login" in out and type(broken).__name__ in out and "crewai login" in out
+
+
 def test_read_last_run_reads_the_record_crewai_writes(tmp_path):
    assert eval_module.read_last_run(tmp_path) is None
    (tmp_path / ".crewai").mkdir()