-
Notifications
You must be signed in to change notification settings - Fork 4
feat: load Infrahub credentials from .env in stdio/.mcp.json mode #147
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: stable
Are you sure you want to change the base?
Changes from all commits
f3d844f
79afb1f
5fda37f
5f0ae83
094d24b
23cbb75
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Load Infrahub credentials from a `.env` file in stdio / `.mcp.json` mode, so the API token can be kept out of a committed `.mcp.json`. Real environment variables take precedence, and `INFRAHUB_MCP_ENV_FILE` overrides the default `./.env` path. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,12 +3,16 @@ | |
| from __future__ import annotations | ||
|
|
||
| import os | ||
| from typing import TYPE_CHECKING | ||
| from unittest.mock import patch | ||
|
|
||
| import pytest | ||
| from pydantic import ValidationError | ||
|
|
||
| from infrahub_mcp.config import ServerConfig, load_config | ||
| from infrahub_mcp.config import ServerConfig, _prime_env_from_dotenv, load_config | ||
|
|
||
| if TYPE_CHECKING: | ||
| from pathlib import Path | ||
|
|
||
|
|
||
| class TestServerConfig: | ||
|
|
@@ -431,3 +435,109 @@ def test_auth_mode_none_ignores_oidc_fields(self) -> None: | |
| config = load_config() | ||
| assert config.auth_mode == "none" | ||
| assert config.oidc_config_url == "https://example.com/.well-known/openid-configuration" | ||
|
|
||
|
|
||
| class TestDotenv: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [review] Existing The ~25 pre-existing Suggest an autouse fixture in this module (or conftest) that points
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 5fda37f: priming was removed from load_config(), so these tests no longer read any .env. An autouse fixture in conftest.py also disables .env probing by default. |
||
| def test_populates_missing_var(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| (tmp_path / ".env").write_text("INFRAHUB_API_TOKEN=from-dotenv\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert os.environ["INFRAHUB_API_TOKEN"] == "from-dotenv" # noqa: S105 | ||
|
|
||
| def test_real_env_wins(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| (tmp_path / ".env").write_text("INFRAHUB_API_TOKEN=from-dotenv\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {"INFRAHUB_API_TOKEN": "from-env"}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert os.environ["INFRAHUB_API_TOKEN"] == "from-env" # noqa: S105 | ||
|
|
||
| def test_real_env_wins_case_insensitively(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # ServerConfig uses case_sensitive=False, so a lowercase real env var must | ||
| # not be shadowed by an uppercase .env entry that resolves to the same key. | ||
| (tmp_path / ".env").write_text("INFRAHUB_API_TOKEN=from-dotenv\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {"infrahub_api_token": "from-env-lower"}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert "INFRAHUB_API_TOKEN" not in os.environ | ||
| assert os.environ["infrahub_api_token"] == "from-env-lower" # noqa: SIM112, S105 | ||
|
|
||
| def test_custom_path_honored(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| custom = tmp_path / "secrets.env" | ||
| custom.write_text("INFRAHUB_API_TOKEN=from-custom\n") | ||
| monkeypatch.chdir(tmp_path) # ensure no ./.env is picked up instead | ||
| with patch.dict(os.environ, {"INFRAHUB_MCP_ENV_FILE": str(custom)}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert os.environ["INFRAHUB_API_TOKEN"] == "from-custom" # noqa: S105 | ||
|
|
||
| def test_missing_file_is_noop(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| monkeypatch.chdir(tmp_path) # empty dir, no .env | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert "INFRAHUB_API_TOKEN" not in os.environ | ||
|
|
||
| def test_ignores_non_infrahub_keys(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # A foreign project .env must not inject unrelated vars (LOG_LEVEL, proxies, secrets). | ||
| (tmp_path / ".env").write_text("LOG_LEVEL=warn\nHTTPS_PROXY=http://x\nINFRAHUB_API_TOKEN=t\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert os.environ["INFRAHUB_API_TOKEN"] == "t" # noqa: S105 | ||
| assert "LOG_LEVEL" not in os.environ | ||
| assert "HTTPS_PROXY" not in os.environ | ||
|
|
||
| def test_ignores_mcp_settings(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # INFRAHUB_MCP_* server settings are NOT sourced from .env: ServerConfig is built | ||
| # at import before priming, so loading them would set values that never take effect. | ||
| (tmp_path / ".env").write_text( | ||
| "INFRAHUB_MCP_READ_ONLY=true\nINFRAHUB_MCP_AUTH_MODE=token-passthrough\nINFRAHUB_API_TOKEN=t\n" | ||
| ) | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert os.environ["INFRAHUB_API_TOKEN"] == "t" # noqa: S105 | ||
| assert "INFRAHUB_MCP_READ_ONLY" not in os.environ | ||
| assert "INFRAHUB_MCP_AUTH_MODE" not in os.environ | ||
|
|
||
| def test_case_variant_within_file_applies_once(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # Case-variant duplicates in the file must not both land in the environment. | ||
| (tmp_path / ".env").write_text("INFRAHUB_API_TOKEN=first\ninfrahub_api_token=second\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert os.environ["INFRAHUB_API_TOKEN"] == "first" # noqa: S105 | ||
| assert "infrahub_api_token" not in os.environ | ||
|
|
||
| def test_empty_path_disables_loading(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| (tmp_path / ".env").write_text("INFRAHUB_API_TOKEN=from-dotenv\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {"INFRAHUB_MCP_ENV_FILE": ""}, clear=True): | ||
| _prime_env_from_dotenv() | ||
| assert "INFRAHUB_API_TOKEN" not in os.environ | ||
|
|
||
| def test_explicit_missing_path_warns( | ||
| self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture | ||
| ) -> None: | ||
| monkeypatch.chdir(tmp_path) | ||
| missing = str(tmp_path / "nope.env") | ||
| with patch.dict(os.environ, {"INFRAHUB_MCP_ENV_FILE": missing}, clear=True), caplog.at_level("WARNING"): | ||
| _prime_env_from_dotenv() | ||
| assert "INFRAHUB_API_TOKEN" not in os.environ | ||
| assert "nope.env" in caplog.text | ||
|
|
||
| def test_unreadable_file_is_noop(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # A non-UTF-8 .env must not crash startup. | ||
| (tmp_path / ".env").write_bytes(b"\xff\xfeINFRAHUB_API_TOKEN=x\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| _prime_env_from_dotenv() # must not raise | ||
| assert "INFRAHUB_API_TOKEN" not in os.environ | ||
|
|
||
| def test_load_config_does_not_read_dotenv(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: | ||
| # Priming happens at server startup, not in load_config(), so importing/loading | ||
| # config never reads a CWD .env or mutates the environment. | ||
| (tmp_path / ".env").write_text("INFRAHUB_API_TOKEN=from-dotenv\n") | ||
| monkeypatch.chdir(tmp_path) | ||
| with patch.dict(os.environ, {}, clear=True): | ||
| load_config() | ||
| assert "INFRAHUB_API_TOKEN" not in os.environ | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[review]
existingsnapshot is not updated inside the loop — the documented case-insensitive invariant breaks within the file itself.A
.envcontaining case-variant duplicates (INFRAHUB_MCP_READ_ONLY=true+infrahub_mcp_read_only=false) injects both spellings intoos.environ; which one pydantic-settings resolves is iteration-order dependent. Verified locally:read_onlycame outFalseeven thoughtrueappeared first in the file.Add applied keys back to the set (
existing.add(key.lower())) as they are written.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 5fda37f: applied keys are added to the existing set inside the loop, so case-variant duplicates within the file are no longer both set. Added a regression test.