diff --git a/README.md b/README.md index c2bb478..d396356 100644 --- a/README.md +++ b/README.md @@ -305,7 +305,7 @@ hai sessions watch hai mcp install ``` -`hai login` signs in through the browser with Google and stores a key in `~/.config/hai/.env`. Without a Google account or a browser, create a key at [platform.hcompany.ai/settings/api-keys](https://platform.hcompany.ai/settings/api-keys) and run `hai login --key`. `hai mcp install` adds the hai-agents MCP server to Cursor, VS Code, Claude Code, and other MCP clients. Credentials resolve from `--api-key`, then `HAI_API_KEY`, then a local `.env`, then `~/.config/hai/.env`. Run `hai --help` for the full command set. +`hai login` signs in through the browser with Google and stores a key in `~/.config/hai/.env`. Without a Google account or a browser, create a key at [platform.hcompany.ai/settings/api-keys](https://platform.hcompany.ai/settings/api-keys) and run `hai login --key`. `hai mcp install` adds the hai-agents MCP server to Cursor, VS Code, Claude Code, and other MCP clients. Credentials resolve from `--api-key`, then `HAI_API_KEY`, then a local `.env`, then `~/.config/hai/.env`. When the key comes from the local `.env`, every command says so on stderr: a cloned or forked repo can ship a `.env` carrying someone else's key, and your runs would land in their account. That is a warning, not a block; read it. The global file is only read when it is a regular file owned by you with mode 600, as `hai login` writes it. Run `hai --help` for the full command set. ## Documentation diff --git a/src/hai_agents_cli/app.py b/src/hai_agents_cli/app.py index 196dc51..525d262 100644 --- a/src/hai_agents_cli/app.py +++ b/src/hai_agents_cli/app.py @@ -779,6 +779,7 @@ def main() -> None: def _state(ctx: typer.Context) -> AppState: if not isinstance(ctx.obj, AppState): raise RuntimeError("CLI state was not initialized.") + _key_file_notices(ctx.obj) return ctx.obj @@ -789,6 +790,20 @@ def _client(state: AppState) -> Client: _raise_cli_error(exc) +_key_file_notices_shown: set[str] = set() + + +def _key_file_notices(state: AppState) -> None: + """Stderr notices, once per process: ignored key files and the project `.env` warning.""" + lines = credentials.key_file_warnings() + if credentials.source(state.api_key) == str(credentials.LOCAL_ENV_PATH): + lines.append(credentials.PROJECT_ENV_WARNING) + for line in lines: + if line not in _key_file_notices_shown: + _key_file_notices_shown.add(line) + err_console.print(escape(line), style="dim") + + def _select_agent(state: AppState, client: Client) -> str: """Pick an agent from the live catalog; require --agent when non-interactive.""" if state.json_output or not (sys.stdin.isatty() and sys.stdout.isatty()): diff --git a/src/hai_agents_cli/doctor.py b/src/hai_agents_cli/doctor.py index c3d1fa3..d4c38ee 100644 --- a/src/hai_agents_cli/doctor.py +++ b/src/hai_agents_cli/doctor.py @@ -23,7 +23,14 @@ class CheckResult: def check_login(api_key: str | None) -> CheckResult: if credentials.current_api_key(api_key): - return CheckResult("login", True, f"API key found ({credentials.source(api_key)})") + source = credentials.source(api_key) + detail = f"API key found ({source})" + if source == str(credentials.LOCAL_ENV_PATH): + detail = f"{detail}. {credentials.PROJECT_ENV_WARNING}" + return CheckResult("login", True, detail) + ignored = credentials.key_file_warnings() + if ignored: + return CheckResult("login", False, "; ".join(ignored), fix="fix the file's owner and mode, or run `hai login`") return CheckResult( "login", False, diff --git a/src/hai_agents_common/credentials.py b/src/hai_agents_common/credentials.py index 97c8913..82ec128 100644 --- a/src/hai_agents_common/credentials.py +++ b/src/hai_agents_common/credentials.py @@ -4,7 +4,8 @@ import contextlib import os -from collections.abc import Callable +import stat +from collections.abc import Callable, Iterator from pathlib import Path from urllib.parse import urljoin @@ -28,6 +29,12 @@ LOCAL_ENV_PATH = Path(".env") GLOBAL_ENV_PATH = Path(os.environ.get("XDG_CONFIG_HOME") or (Path.home() / ".config")) / "hai" / ".env" +# Detection, not prevention: a project `.env` is used as is, and the CLI says so every time. +PROJECT_ENV_WARNING = ( + f"Using {API_KEY_VAR} from {LOCAL_ENV_PATH}. Make sure this key is yours: a cloned or forked repo can ship " + "a .env that carries someone else's key on purpose, and your runs would then land in their account." +) + def portal_base(base_url: str | None = None) -> str: """Portal origin used by `hai login`: `HAI_PORTAL_URL`, else the portal of the platform's region.""" @@ -105,12 +112,61 @@ def source(explicit: ApiKey | None = None) -> str | None: return "argument" if os.environ.get(API_KEY_VAR): return "environment" - for path in _env_paths(): - if path.exists() and dotenv_values(path).get(API_KEY_VAR): + for path in _readable_env_paths(): + if _values(path).get(API_KEY_VAR): return str(path) return None +def key_file_rejection(path: Path) -> str | None: + """Why a key file must not be read, or None. + + The global file must be private (`hai login` writes it 0600); a project `.env` only has to be a regular file. + """ + try: + info = os.lstat(path) + except FileNotFoundError: + return None + except OSError as exc: + return str(exc) + if not stat.S_ISREG(info.st_mode): + return "not a regular file (symlink?)" + if path == LOCAL_ENV_PATH or os.name == "nt": + return None + if info.st_uid != os.geteuid(): + return "not owned by the current user" + if info.st_mode & 0o077: + return "readable by others; run `chmod 600` on it" + return None + + +def key_file_warnings() -> list[str]: + """One line per key file that is present but ignored, for the CLI to show.""" + warnings = [] + for path in _env_paths(): + if not os.path.lexists(path): + continue + reason = key_file_rejection(path) # what the file is, before reading what it contains + if reason is None and path == LOCAL_ENV_PATH and not project_env_settings(path): + continue + if reason: + warnings.append(f"Ignoring {path}: {reason}") + return warnings + + +def project_env_settings(path: Path) -> dict[str, str]: + """The `HAI_` variables a project `.env` sets; a `.env` without any is not a key file and is left alone.""" + return {name: value for name, value in _values(path).items() if name.startswith("HAI_") and value} + + +def _values(path: Path) -> dict[str, str | None]: + """The file's variables, or nothing when it cannot be read or is not text.""" + try: + return dotenv_values(path) + except (OSError, UnicodeDecodeError): + return {} + + def _client_kwargs(api_key: ApiKey | None, base_url: str | None) -> dict[str, ApiKey | str]: kwargs: dict[str, ApiKey | str] = {"api_key": resolve_api_key(api_key)} resolved_base_url = resolve_base_url(base_url) @@ -124,13 +180,20 @@ def _env_paths() -> tuple[Path, ...]: return (LOCAL_ENV_PATH, GLOBAL_ENV_PATH) +def _readable_env_paths() -> Iterator[Path]: + for path in _env_paths(): + if not os.path.lexists(path) or key_file_rejection(path) is not None: + continue + if path == LOCAL_ENV_PATH and not project_env_settings(path): + continue + yield path + + def _lookup(name: str) -> str | None: if os.environ.get(name): return os.environ[name] - for path in _env_paths(): - if not path.exists(): - continue - value = dotenv_values(path).get(name) + for path in _readable_env_paths(): + value = _values(path).get(name) if value: return value return None diff --git a/tests/test_credentials.py b/tests/test_credentials.py index d753833..fba433d 100644 --- a/tests/test_credentials.py +++ b/tests/test_credentials.py @@ -20,6 +20,7 @@ def isolated_env(tmp_path, monkeypatch): monkeypatch.delenv(var, raising=False) monkeypatch.setattr(credentials, "LOCAL_ENV_PATH", tmp_path / "local.env") monkeypatch.setattr(credentials, "GLOBAL_ENV_PATH", tmp_path / "global.env") + monkeypatch.setattr(app_module, "_key_file_notices_shown", set()) # the once-per-process memo def test_env_var_beats_dotenv(monkeypatch): @@ -125,3 +126,109 @@ def get_session_quota(self): def _error_text(result) -> str: return "\n".join(part for part in (result.output, result.stderr, str(result.exception)) if part) + + +def _private(path, content): + path.write_text(content) + path.chmod(0o600) + + +def test_project_env_is_used_with_a_warning_once(monkeypatch): + credentials.LOCAL_ENV_PATH.write_text("HAI_API_KEY=hk-local\n") # umask mode, like any checked-out file + + result = runner.invoke(app, ["whoami"]) + + assert result.exit_code == 0, _error_text(result) + text = result.output.replace("\n", "") # output already interleaves stderr; count there only + assert text.count("Make sure this key is yours") == 1 + assert "someone else's key" in text and "hk-local" not in text + + +def test_no_warning_for_the_environment_variable_or_the_global_file(monkeypatch): + _private(credentials.GLOBAL_ENV_PATH, "HAI_API_KEY=hk-global\n") + assert "Make sure this key is yours" not in _unwrapped(runner.invoke(app, ["whoami"])) + + monkeypatch.setenv("HAI_API_KEY", "hk-env") + assert "Make sure this key is yours" not in _unwrapped(runner.invoke(app, ["whoami"])) + + +def test_unrelated_project_env_is_left_alone(): + _private(credentials.GLOBAL_ENV_PATH, "HAI_API_KEY=hk-global\n") + credentials.LOCAL_ENV_PATH.write_text("DATABASE_URL=postgres://x\nFOO=bar\n") + + assert credentials.resolve_api_key() == "hk-global" + assert credentials.key_file_warnings() == [] + assert "Make sure" not in _unwrapped(runner.invoke(app, ["whoami"])) + + +def test_world_readable_global_file_is_ignored(): + """`hai login` writes the global file 0600; anything looser is not ours to trust.""" + credentials.GLOBAL_ENV_PATH.write_text("HAI_API_KEY=hk-global\n") + credentials.GLOBAL_ENV_PATH.chmod(0o644) + + with pytest.raises(RuntimeError, match="No API key found"): + credentials.resolve_api_key() + assert credentials.key_file_warnings() == [ + f"Ignoring {credentials.GLOBAL_ENV_PATH}: readable by others; run `chmod 600` on it" + ] + + +def test_symlinked_project_env_is_ignored(tmp_path): + target = tmp_path / "elsewhere.env" + target.write_text("HAI_API_KEY=hk-elsewhere\n") + credentials.LOCAL_ENV_PATH.symlink_to(target) + + with pytest.raises(RuntimeError, match="No API key found"): + credentials.resolve_api_key() + assert "not a regular file" in credentials.key_file_warnings()[0] + + +def test_doctor_repeats_the_warning_for_a_project_key(): + from hai_agents_cli import doctor + + credentials.LOCAL_ENV_PATH.write_text("HAI_API_KEY=hk-local\n") + + check = doctor.check_login(None) + + assert check.ok is True and "Make sure this key is yours" in check.detail + + +def _unwrapped(result) -> str: + """Console text with Rich's soft line wraps undone, for matching long sentences.""" + return _error_text(result).replace("\n", "") + + +def test_every_command_warns_about_a_project_key_even_in_json_mode(): + credentials.LOCAL_ENV_PATH.write_text("HAI_API_KEY=hk-local\n") + + result = runner.invoke(app, ["--json", "whoami"]) + + assert result.exit_code == 0, _error_text(result) + assert "Make sure this key is yours" in _unwrapped(result) + assert "hk-local" not in _error_text(result) + + +def test_a_binary_project_env_is_ignored_instead_of_crashing(): + credentials.LOCAL_ENV_PATH.write_bytes(b"\xff\xfe\x00not text\x00") + + result = runner.invoke(app, ["whoami"], env={"HAI_API_KEY": "hk-env"}) + + assert result.exit_code == 0, _error_text(result) + assert credentials.key_file_warnings() == [] + + +def test_a_symlinked_project_env_is_never_read(monkeypatch, tmp_path): + """The shortcut check runs before the content check, so a link to /dev/zero cannot hang the CLI.""" + target = tmp_path / "elsewhere.env" + target.write_text("HAI_API_KEY=hk-elsewhere\n") + credentials.LOCAL_ENV_PATH.symlink_to(target) + monkeypatch.setattr(credentials, "dotenv_values", lambda *_: pytest.fail("the symlink must not be read")) + + assert credentials.key_file_warnings() == [f"Ignoring {credentials.LOCAL_ENV_PATH}: not a regular file (symlink?)"] + assert credentials.current_api_key() is None + + +def test_a_dangling_symlink_is_reported(): + credentials.LOCAL_ENV_PATH.symlink_to(credentials.LOCAL_ENV_PATH.parent / "missing.env") + + assert "not a regular file" in credentials.key_file_warnings()[0]