Skip to content

Harden load_env_file and resolve_env: export-prefix lines, quote stripping, and silent api_key_env misses #759

Description

@huangzesen

Summary

load_env_file in src/lingtai_kernel/config_resolve.py mishandles two common .env idioms: export KEY=val lines pollute os.environ with a bogus "export KEY" name (leaving KEY unset), and val.strip("'\"") strips arbitrary runs of mixed quote characters from both ends, corrupting values that legitimately begin or end with a quote. Separately, resolve_env silently falls back when the named variable is unset or empty, so an api_key_env that points at a variable missing from the env_file yields api_key=None with zero diagnostics — the agent boots and only fails later, deep inside the first LLM call.

Narrative

An agent's init.json typically configures its LLM credentials indirectly: the manifest carries "api_key_env": "ANTHROPIC_API_KEY" (each provider's defaults module ships one, e.g. src/lingtai/llm/anthropic/defaults.py), and a top-level "env_file" points at a dotenv-style file the kernel loads at boot. The loader is deliberately minimal — no python-dotenv dependency — and lives in src/lingtai_kernel/config_resolve.py:51-73:

key, _, val = line.partition("=")
if not _:
    continue
key = key.strip()
val = val.strip().strip("'\"")
if overwrite or key not in os.environ:
    os.environ[key] = val

Now consider the most common way a user produces an env file: they copy lines out of their ~/.zshrc or a shell snippet from a provider's docs. Those lines look like export ANTHROPIC_API_KEY=sk-ant-.... The partition("=") gives key = "export ANTHROPIC_API_KEY", which key.strip() does not fix (the space is internal). The loader then sets os.environ["export ANTHROPIC_API_KEY"] — a name no lookup will ever find — and the variable the manifest actually names, ANTHROPIC_API_KEY, remains unset. Every mainstream dotenv implementation (python-dotenv, docker compose, direnv) accepts the export prefix precisely because this copy-paste path is so common.

The failure then compounds because nothing downstream notices. resolve_env (src/lingtai_kernel/config_resolve.py:42-48) is a silent best-effort helper:

def resolve_env(value: str | None, env_name: str | None) -> str | None:
    if env_name:
        env_val = os.environ.get(env_name)
        if env_val:
            return env_val
    return value

If os.environ.get(env_name) returns None (or empty string), it falls back to value — which for the api-key path is llm.get("api_key"), usually None. Both boot paths consume this without checking: src/lingtai/cli.py:99 (api_key = resolve_env(llm.get("api_key"), llm.get("api_key_env"))) and the refresh path at src/lingtai/agent.py:1244. Schema validation offers no safety net either: src/lingtai/init_schema.py:349-355 only verifies that an env_file key exists in the config when api_key_env is set without api_key — it never checks that the file exists on disk, let alone that the named variable actually resolves after loading it. So the user's experience is: agent starts cleanly, greets them, and then the first real turn dies inside the provider adapter with an authentication error (or a confusing "no API key" exception several stack frames removed from the actual cause: a stray export prefix in a text file).

The quote-stripping bug is subtler but corrupts data rather than dropping it. str.strip("'\"") treats its argument as a set of characters, removing any run of ' and " from both ends independently. KEY='"hello"' (single quotes wrapping a value that itself contains double quotes — a standard dotenv way to preserve inner quotes) loses both layers and becomes hello. An unquoted password or token that happens to end in " gets truncated. A value like KEY="abc (unbalanced, arguably user error) silently loses its leading quote instead of being kept verbatim or flagged. Correct dotenv semantics strip exactly one layer of matching, symmetric quotes.

The current design got here honestly: a tiny hand-rolled parser avoids a dependency, and resolve_env is generic — it also serves _resolve_env_fields for capability kwargs (src/lingtai_kernel/config_resolve.py:85-92), where a missing env var is often legitimately optional. Better looks like: teach the parser the two dotenv idioms it's missing (a few lines each), keep resolve_env itself silent for the generic path, and add an explicit diagnostic at the one call site where a miss is always a misconfiguration — the LLM api-key resolution at agent boot/refresh.

Current behavior

  • load_env_file (src/lingtai_kernel/config_resolve.py:51-73):
    • A line export KEY=val sets os.environ["export KEY"] = "val"; KEY is never set. No warning.
    • val.strip().strip("'\"") removes any run of ' and " characters from both ends, asymmetrically and across mixed quote styles. '"v"'v (both layers stripped); abc"abc (data loss); "abcabc (unbalanced quote silently eaten).
  • resolve_env (src/lingtai_kernel/config_resolve.py:42-48): returns the fallback value when env_name is unset in — or set to empty in — os.environ, with no diagnostic. Note it also treats an explicitly-empty env var as "unset" (the if env_val: truthiness check), which matters for the diagnostic wording.
  • init_schema.validate_init (src/lingtai/init_schema.py:349-355): when api_key_env is set without api_key, it only requires that data["env_file"] is present in the config dict. It does not verify the file exists or that the variable resolves.
  • Boot (src/lingtai/cli.py:89-99, build_agent) and refresh (src/lingtai/agent.py:1180-1244, _setup_from_init) both call load_env_file then resolve_env(llm.get("api_key"), llm.get("api_key_env")) and pass a possibly-None api_key straight into LLMService with no check.

Detailed implementation plan

  1. Fix the parser in load_env_file (src/lingtai_kernel/config_resolve.py). Replace the body of the per-line handling:

    for line in env_path.read_text(encoding="utf-8").splitlines():
        line = line.strip()
        if not line or line.startswith("#"):
            continue
        if line.startswith("export ") or line.startswith("export\t"):
            line = line[len("export"):].lstrip()
        key, sep, val = line.partition("=")
        if not sep:
            continue
        key = key.strip()
        if not key or " " in key:
            continue  # malformed line; never write a bogus env name
        val = _strip_matched_quotes(val.strip())
        if overwrite or key not in os.environ:
            os.environ[key] = val
  2. Add a module-private helper _strip_matched_quotes(val: str) -> str in the same file: strip exactly one layer of symmetric quotes, only when both ends match and the value is at least 2 chars:

    def _strip_matched_quotes(val: str) -> str:
        if len(val) >= 2 and val[0] == val[-1] and val[0] in ("'", '"'):
            return val[1:-1]
        return val

    Do not recurse — '"v"' should yield "v" (one layer), matching dotenv conventions.

  3. Add a resolution guard for the api-key path. Add a small public helper in src/lingtai_kernel/config_resolve.py so both boot paths share it:

    def resolve_env_checked(value, env_name, *, context="", warn=None):
        """resolve_env plus a diagnostic when env_name is named but does not resolve."""
        resolved = resolve_env(value, env_name)
        if env_name and resolved is None:
            msg = (f"{context}: environment variable {env_name!r} is unset or empty "
                   f"and no fallback value is configured")
            (warn or (lambda m: print(f"warning: {m}", file=sys.stderr)))(msg)
        return resolved

    Leave resolve_env itself untouched — _resolve_env_fields and the capability kwargs paths (src/lingtai/capabilities/web_search/__init__.py, vision, daemon preset resolution) legitimately tolerate misses.

  4. Use it at the two api-key call sites. In src/lingtai/cli.py (build_agent, currently line 99) replace the resolve_env call with resolve_env_checked(..., context="manifest.llm.api_key_env"). In src/lingtai/agent.py (_setup_from_init, currently line 1244) do the same, passing warn=lambda m: self._log("env_resolve_warning", message=m) so the diagnostic lands in the agent log. Decide warn-vs-raise: recommend warn at refresh (a live agent should not be killed by a hot-edited env_file) and raise ValueError at initial boot in cli.py — a fresh agent with no key can never make an LLM call, and failing fast at boot with a message naming the variable and the env_file path is strictly better than an opaque provider auth error later. Exempt providers that intentionally have no key (e.g. claude_code, whose defaults omit api_key_env per src/lingtai/llm/claude_code/defaults.py) — the guard is keyed on api_key_env being set, so they are naturally unaffected.

  5. Strengthen the boot-time check in src/lingtai/init_schema.py (around line 349): keep the existing structural rule, and optionally extend the warnings list (the function already accumulates non-fatal warnings) when env_file is set but the path does not exist on disk at validation time. Keep this a warning, not an error, since validation can run in contexts where relative paths are not yet resolved (resolve_paths in config_resolve.py runs against working_dir separately).

  6. Back-compat: the export-prefix and matched-quote changes only alter behavior for inputs that are broken today (bogus "export KEY" names, mixed-quote corruption), so no migration is needed. The one observable change: values previously written as KEY=''something'' (double-layered same-style quotes) will now keep one quote layer — this matches dotenv and is the correct reading. resolve_env's signature and semantics are unchanged for all existing callers.

Risks and alternatives

  • Adopting python-dotenv instead would fix parsing wholesale, but adds a dependency to a kernel package that is deliberately lean, and its interpolation/multiline features exceed what init.json's contract promises. The ~10-line hand fix keeps the surface auditable.
  • Raising instead of warning inside resolve_env itself would break the capability-kwargs paths (_resolve_env_fields at src/lingtai_kernel/config_resolve.py:85-92) where absent env vars are routine. Hence the separate resolve_env_checked wrapper applied only where a miss is definitely wrong.
  • Raising at refresh (agent.py:_setup_from_init) could brick a running agent whose env_file was mid-edit; warn-and-continue preserves the existing "refresh is forgiving" posture while still surfacing the cause in the log.
  • Skipping malformed keys silently (step 1's " " in key guard) could hide typos; acceptable because the previous behavior (writing garbage into os.environ) was strictly worse, and a debug-level note can be added if desired.
  • Values that intentionally end with a quote and were previously being truncated will now round-trip correctly — any downstream consumer that had compensated for the truncation (unlikely) would notice. No such compensation exists in-repo.

Testing plan

Extend tests/test_cli.py (which already hosts test_load_env_file, test_load_env_file_missing, test_resolve_env_prefers_env_var) or add a dedicated tests/test_config_resolve.py:

  • test_load_env_file_export_prefix — file containing export FOO=bar sets os.environ["FOO"] == "bar" and does not create any key containing a space.
  • test_load_env_file_export_tab_prefixexport\tFOO=bar also parses.
  • test_load_env_file_matched_quotes_single_layerA="v", B='v', C='"v"' yield v, v, "v" respectively (one layer only).
  • test_load_env_file_unmatched_quote_preservedA=abc" keeps the trailing quote; B="abc keeps the leading quote.
  • test_load_env_file_malformed_key_skippedsome words=x writes nothing to os.environ.
  • test_resolve_env_checked_warns_on_miss — with MISSING_VAR absent, resolve_env_checked(None, "MISSING_VAR", context="t", warn=capture) returns None and the capture receives a message naming MISSING_VAR.
  • test_resolve_env_checked_silent_on_hit_and_fallback — no warning when the env var resolves, and none when env_name is None.
  • test_build_agent_raises_on_unresolvable_api_key_env — a tmp agent dir whose env_file omits the variable named by manifest.llm.api_key_env makes build_agent raise with the variable name in the message (or, if warn-only is chosen for boot too, asserts the warning).
  • Refresh path: in tests/test_deep_refresh.py, add test_refresh_logs_env_resolve_warning asserting the agent log receives env_resolve_warning when the env_file loses the key between boots.
  • test_validate_init_warns_missing_env_file_on_diskvalidate_init appends a warning (not an error) when env_file points at a nonexistent absolute path.

Generated by a Claude Fable 5 deep review of the lingtai-kernel codebase.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requestfable-5Filed by Claude Fable 5 repo review

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions