feat: load Infrahub credentials from .env in stdio/.mcp.json mode - #147
feat: load Infrahub credentials from .env in stdio/.mcp.json mode#147iddocohen wants to merge 6 commits into
Conversation
Deploying infrahub-mcp-server with
|
| Latest commit: |
23cbb75
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://954f875f.infrahub-mcp-server.pages.dev |
| Branch Preview URL: | https://ic-feat-ic-env-read-7r870.infrahub-mcp-server.pages.dev |
There was a problem hiding this comment.
All reported issues were addressed across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…mode Prime os.environ from a .env file at the top of load_config() (before ServerConfig and any InfrahubClient() are built), so the API token can be kept out of a committed .mcp.json. Real environment variables win over the file, case-insensitively to match ServerConfig(case_sensitive=False); the path defaults to ./.env and is overridable via INFRAHUB_MCP_ENV_FILE.
6a0cc06 to
79afb1f
Compare
|
|
||
| path = os.environ.get("INFRAHUB_MCP_ENV_FILE") or ".env" | ||
| existing = {key.lower() for key in os.environ} | ||
| for key, value in dotenv_values(path).items(): |
There was a problem hiding this comment.
[review] Foreign .env with LOG_LEVEL crashes server startup.
This loop copies every key from .env, not just INFRAHUB_* ones. ServerConfig.log_level has validation_alias=AliasChoices("log_level", ...) with case_sensitive=False, so a bare LOG_LEVEL env var binds to the field. Verified locally: a .env containing LOG_LEVEL=warn (common in web-project .env files) in the launch directory makes load_config() raise INFRAHUB_MCP_LOG_LEVEL must be one of [...], got 'warn' — the MCP server fails to start in any repo whose .env has LOG_LEVEL=warn|trace|verbose, citing a variable the user never set. Even a valid LOG_LEVEL=debug silently flips the server to debug logging.
Suggested fix: only copy keys with the INFRAHUB_ prefix (covers both INFRAHUB_MCP_* and the SDK's INFRAHUB_*).
There was a problem hiding this comment.
Fixed in 5fda37f: the loader now copies only INFRAHUB_-prefixed keys, so LOG_LEVEL and other unrelated variables are never applied and can no longer crash or silently reconfigure startup.
| path = os.environ.get("INFRAHUB_MCP_ENV_FILE") or ".env" | ||
| existing = {key.lower() for key in os.environ} | ||
| for key, value in dotenv_values(path).items(): | ||
| if value is not None and key.lower() not in existing: |
There was a problem hiding this comment.
[review] Unscoped .env loading injects unrelated variables and secrets into the server process.
Because all keys are applied, a foreign project .env can inject HTTP_PROXY/HTTPS_PROXY/SSL_CERT_FILE/REQUESTS_CA_BUNDLE — httpx (the SDK transport) honors these by default, so Infrahub connectivity silently breaks or gets routed through a proxy. Unrelated secrets (OPENAI_API_KEY, DATABASE_URL, ...) also become part of this process environment and are inherited by any subprocess.
Same fix as the LOG_LEVEL issue: scope the copy to INFRAHUB_-prefixed keys.
There was a problem hiding this comment.
Fixed in 5fda37f by the same scoping: only INFRAHUB_-prefixed keys are copied, so proxy settings and third-party secrets in a foreign .env are ignored.
| assert config.oidc_config_url == "https://example.com/.well-known/openid-configuration" | ||
|
|
||
|
|
||
| class TestDotenv: |
There was a problem hiding this comment.
[review] Existing TestLoadConfig tests now depend on an untracked repo-root .env.
The ~25 pre-existing TestLoadConfig tests call load_config() without monkeypatch.chdir, so they now read ./.env from the pytest CWD (the repo root). The docs added in this PR tell developers to create exactly that file; one containing INFRAHUB_MCP_READ_ONLY=true makes test_defaults_no_env fail, and LOG_LEVEL=warn makes every load_config() call in the suite raise — failing locally while CI stays green.
Suggest an autouse fixture in this module (or conftest) that points INFRAHUB_MCP_ENV_FILE at a nonexistent path (or chdirs to tmp_path) for the non-dotenv tests.
There was a problem hiding this comment.
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 load_config() -> ServerConfig: | ||
| """Load and validate server configuration from environment variables.""" | ||
| _prime_env_from_dotenv() |
There was a problem hiding this comment.
[review] .env loading now happens at library import time.
server.py:50 runs load_config() at module import, and import infrahub_mcp reaches it via __init__ → _cli → server. Importing the package now reads a CWD-relative file and mutates os.environ for the whole process. Concretely: during pytest collection, importing test_health.py/test_tools.py/test_cli.py primes the real environment from the repo-root .env before any test or fixture runs; any application embedding this library gets the same silent env mutation on import.
Consider moving the priming to process entry (main() / lifespan) instead of the import-time load_config() path.
There was a problem hiding this comment.
Fixed in 5fda37f: priming moved from the import-time load_config() path into the server lifespan, so importing the package no longer reads the filesystem or mutates os.environ. Verified with a standalone import.
|
|
||
| path = os.environ.get("INFRAHUB_MCP_ENV_FILE") or ".env" | ||
| existing = {key.lower() for key in os.environ} | ||
| for key, value in dotenv_values(path).items(): |
There was a problem hiding this comment.
[review] Only a missing .env is a no-op — an unreadable one crashes startup.
Verified locally: an existing .env without read permission makes load_config() raise PermissionError: [Errno 13] (python-dotenv only guards isfile, then open() propagates). A non-UTF-8 file raises UnicodeDecodeError (strict utf-8 decode), and a FIFO named .env blocks open() indefinitely. Since probing is default-on, a root-owned .env in the launch dir (e.g. created by docker-compose) crashes the MCP server with a raw traceback even though the user never opted in.
Wrap the read in try/except (OSError, UnicodeDecodeError) with a clear log message.
There was a problem hiding this comment.
Fixed in 5fda37f: the read is wrapped in try/except (OSError, UnicodeDecodeError) with a warning, and non-regular files are skipped, so an unreadable or non-UTF-8 .env no longer crashes startup.
| """ | ||
| from dotenv import dotenv_values # noqa: PLC0415 | ||
|
|
||
| path = os.environ.get("INFRAHUB_MCP_ENV_FILE") or ".env" |
There was a problem hiding this comment.
[review] No way to disable .env loading; an explicit missing path fails silently.
os.environ.get("INFRAHUB_MCP_ENV_FILE") or ".env" means INFRAHUB_MCP_ENV_FILE="" falls back to the default probe, so loading cannot be turned off (only redirected to a nonexistent path as a workaround). And when the variable is explicitly set to a path that does not exist (typo), dotenv_values returns {} silently (verbose=False), so the user only sees a generic "Authentication required" later, with no hint that their secrets file was never read.
Suggest: treat empty as "disabled", and log a warning when an explicitly configured path is missing.
There was a problem hiding this comment.
Fixed in 5fda37f: an empty INFRAHUB_MCP_ENV_FILE now disables loading, and an explicitly configured path that does not exist logs a warning.
| from dotenv import dotenv_values # noqa: PLC0415 | ||
|
|
||
| path = os.environ.get("INFRAHUB_MCP_ENV_FILE") or ".env" | ||
| existing = {key.lower() for key in os.environ} |
There was a problem hiding this comment.
[review] existing snapshot is not updated inside the loop — the documented case-insensitive invariant breaks within the file itself.
A .env containing case-variant duplicates (INFRAHUB_MCP_READ_ONLY=true + infrahub_mcp_read_only=false) injects both spellings into os.environ; which one pydantic-settings resolves is iteration-order dependent. Verified locally: read_only came out False even though true appeared 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.
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.
| `.env` applies to the default stdio / `.mcp.json` setup (`auth_mode=none`). In | ||
| the HTTP `token-passthrough` and `basic-passthrough` modes the per-request | ||
| credential comes from the HTTP `Authorization` header, so `.env` there only | ||
| supplies `INFRAHUB_ADDRESS`. |
There was a problem hiding this comment.
[review] Inaccurate claim: in passthrough modes .env supplies more than INFRAHUB_ADDRESS.
The loader applies every variable that is not already set, in every auth mode — including INFRAHUB_MCP_* behavior flags (read_only, rate limits, even auth_mode). A .env in the HTTP server working directory can therefore change server behavior well beyond the address. Worth rewording so operators of HTTP deployments are not misled.
There was a problem hiding this comment.
Fixed in 5fda37f: reworded. The loader is scoped to INFRAHUB_ keys and no longer changes INFRAHUB_MCP_ behavior flags, and the note now clarifies that in passthrough modes the token comes from the Authorization header.
Addresses review feedback: - Copy only INFRAHUB_-prefixed keys, so a foreign project .env cannot inject LOG_LEVEL, proxy settings, or unrelated secrets into the process. - Move priming out of load_config() into the server lifespan, so importing the package never reads the filesystem or mutates os.environ (and the existing load_config tests no longer depend on a repo-root .env). - Track applied keys so case-variant duplicates within the file are not both set. - Guard the read with try/except (OSError, UnicodeDecodeError) and a warning. - Empty INFRAHUB_MCP_ENV_FILE disables loading; an explicit missing path warns. - Add an autouse test fixture disabling .env probing by default. - Reword docs to reflect INFRAHUB_-only scope and passthrough behavior.
There was a problem hiding this comment.
All reported issues were addressed across 6 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…AHUB_MCP_* Priming runs at server startup, after ServerConfig is snapshotted at import, so INFRAHUB_MCP_* values in a .env would never take effect. Rather than reload config at runtime (which would let a stray .env silently change auth_mode or read_only), scope .env to the SDK connection variables (INFRAHUB_ prefix excluding INFRAHUB_MCP_) and document that MCP server settings come from the real environment or the .mcp.json env block.
…cs build clean locally and in CI)
What
Adds
.envfile support so that in the stdio /.mcp.jsonsetup (no HTTP server), the Infrahub API token can be supplied from a gitignored.envfile instead of being hardcoded in a committed.mcp.json.Why
.mcp.jsonis typically committed to the consumer repository, so puttingINFRAHUB_API_TOKENin itsenv:block leaks the secret. With this change,.mcp.jsonkeeps only the non-secretINFRAHUB_ADDRESS, and the token lives in a gitignored.envnext to it.How
load_config()calls a new_prime_env_from_dotenv()at import time (beforeServerConfigand anyInfrahubClient()are constructed). It runs:Because both credential layers already read
os.environ— the MCPServerConfig(INFRAHUB_MCP_*) and the Infrahub SDK client (INFRAHUB_*) — priming the environment once is sufficient. No changes toauth.py,utils.get_client(), or any SDK call site..mcp.jsonenv:block or shell export) always win over the file (override=False).INFRAHUB_MCP_ENV_FILE=/path/to/fileoverrides the default./.env. Read raw fromos.environ(not aServerConfigfield) because it must resolve before the config model exists. No walk-up.Interaction with auth modes
.envsupport is inherently theauth_mode=none/ stdio /.mcp.jsoncase. In HTTPtoken-passthrough/basic-passthroughmodes the per-request credential comes from theAuthorizationheader, so.envthere only suppliesINFRAHUB_ADDRESS. OIDC is a server-operator concern. No auth logic changed.Changes
pyproject.toml— declarepython-dotenv>=1.0,<2.src/infrahub_mcp/config.py—_prime_env_from_dotenv(), called first inload_config().tests/unit/test_config.py— 5 tests: missing-var population, real-env-wins, custom path, missing-file no-op, end-to-end throughload_config()..envworkflow in the Claude Code integration guide;INFRAHUB_MCP_ENV_FILEin the configuration reference.docs/superpowers/.Testing
uv run pytest→ 336 passed, 23 skipped.uv run invoke lint→ ruff/mypy/yamllint clean, pylint 10.00/10.Summary by cubic
Loads Infrahub credentials from a gitignored
.envfor stdio/.mcp.jsonsetups, keeping tokens out of version control. HTTP passthrough modes are unchanged;.envthere only suppliesINFRAHUB_ADDRESS.New Features
.envat server startup (lifespan), not inload_config(); importing or loading config doesn’t read the filesystem.INFRAHUB_ADDRESS,INFRAHUB_API_TOKEN,INFRAHUB_USERNAME,INFRAHUB_PASSWORD); ignore non-INFRAHUB_*keys and allINFRAHUB_MCP_*. Real env vars win case-insensitively; case-variant duplicates in.envapply once.INFRAHUB_MCP_ENV_FILEsets a custom path; empty disables loading. Missing default is a no-op; explicit missing path and read errors log a warning.Dependencies
python-dotenv>=1.0,<2.Written for commit 23cbb75. Summary will update on new commits.