Skip to content

fix(providers): openai-compatible sends an empty api_key, so the documented env-var fallback never happens #439

Description

@pjdoland

Summary

The admin guide promises that an operator can leave the API key out of config.json and have NBI pick it up from the provider's standard environment variable. For the openai-compatible provider that does not happen: the provider sends an empty string, which suppresses the OpenAI SDK's own environment lookup, and the request fails with a missing-credentials error instead.

The claim

docs/admin-guide.md, "API-key handling":

Inject the org's keys via env vars at pod startup. Set the provider's expected env var (OPENAI_API_KEY, ANTHROPIC_API_KEY, etc.) on the pod. Configure the provider in <env-prefix>/share/jupyter/nbi/config.json without a key — NBI picks up the provider's standard env var.

This is the recommended approach for multi-tenant clusters, and the same section says even the env-prefix base config should not contain keys.

Why it fails

The property's default value is an empty string:

# notebook_intelligence/llm_providers/openai_compatible_llm_provider.py
LLMProviderProperty("api_key", "API key", "API key", "", False),

and it is passed through unconditionally:

base_url_prop = self.get_property("base_url")
base_url = base_url_prop.value if base_url_prop is not None else None
base_url = base_url if base_url.strip() != "" else None   # <- empty coerced to None
api_key = self.get_property("api_key").value              # <- no such coercion

client = OpenAI(base_url=base_url, api_key=api_key)

The SDK only consults the environment when the key is None:

# openai/_client.py
if api_key is None:
    api_key = os.environ.get("OPENAI_API_KEY")

So an unset key arrives as "", which is not None, the fallback is skipped, and the client is constructed with an empty credential.

Note the asymmetry two lines apart: base_url already gets the empty-to-None treatment, api_key does not.

Impact

Admin-facing and silent until first use. An operator follows the documented cluster approach, ships a base config with no key, injects OPENAI_API_KEY on the pod, and every chat turn fails authentication. The config looks correct and the guide says it should work.

Proposed direction

Coerce an empty key to None at both call sites, mirroring what base_url already does, so the SDK's documented fallback applies:

api_key_prop = self.get_property("api_key")
api_key = api_key_prop.value if api_key_prop is not None else None
api_key = api_key if api_key and api_key.strip() != "" else None

That makes the existing documentation true rather than changing it. Both classes in the file need it (OpenAICompatibleChatModel and OpenAICompatibleInlineCompletionModel).

Worth checking as part of the same fix: whether litellm-compatible has the same gap. Its upstream uses api_key or get_secret(...) patterns in places, where a falsy "" does chain to a secret lookup, so it may already behave as documented; that was not traced end to end.

Provenance

Found while correcting a different documentation defect in the same guide (#438). Filed separately because this one is a code fix that makes existing documentation correct, not a documentation change.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions