Adam SmithandClaude Sonnet 5
b96b46495a
Critical (review finding 1): async_step_reauth_confirm showed STEP_USER_DATA_SCHEMA directly instead of prefilling it via add_suggested_values_to_schema (unlike async_step_user, which does), so the reauth form displayed DEFAULT_BASE_URL rather than the entry's configured endpoint. A user rotating their API key would have their integration silently repointed from e.g. LiteLLM back to api.openai.com on save. Fixed by prefilling from self._get_reauth_entry().data, same as the user step. strings.json's reauth_confirm block now documents the base_url field too. Regression test: test_reauth_preserves_base_url (asserts both the shown form's suggested_value and the final stored base_url). Important (review finding 2): base_url is user-suppliable and could carry embedded user:pass@ credentials that later surface through vendored exception logging (e.g. entity.py's "Error talking to OpenAI: %s") this fork does not and should not edit. Rather than touch vendored logging, reject credentials at the boundary: normalize_base_url now raises the new CredentialsInBaseURL (a subclass of InvalidBaseURL) when the URL has a username or password, mapped to a distinct "credentials_in_url" form error telling the user to use the API key field instead. Tests added in test_url_util.py and test_config_flow.py. Important (review finding 3): documented on sanitize_url_for_logging that it is not a general-purpose credential scrubber -- it only masks what urlparse recognizes as a netloc's user:pass@ authority component, so a scheme-less/relative string with embedded credentials passes through unmasked. Safe today because every caller normalizes first (which now also rejects embedded credentials outright), but it is exported for a later task's use and must not be assumed bulletproof on an arbitrary unvalidated string. Not fixed (reviewer's Minor 4, by direction): the default-config setup call reaching api.openai.com with Authorization: Bearer not-required when both base_url and api_key are left at defaults -- cosmetic, and correctly surfaces as invalid_auth. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01739Seac5eMAfNwFeiwV5r4
61 lines
1.7 KiB
Python
61 lines
1.7 KiB
Python
"""URL hygiene: normalize input, never log credentials."""
|
|
|
|
import pytest
|
|
|
|
from custom_components.openai_compatible.url_util import (
|
|
CredentialsInBaseURL,
|
|
InvalidBaseURL,
|
|
normalize_base_url,
|
|
sanitize_url_for_logging,
|
|
)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
("raw", "expected"),
|
|
[
|
|
("http://192.0.2.10:4000/v1", "http://192.0.2.10:4000/v1"),
|
|
("http://192.0.2.10:4000/v1/", "http://192.0.2.10:4000/v1"),
|
|
(" https://api.openai.com/v1 ", "https://api.openai.com/v1"),
|
|
("http://host:4000/v1///", "http://host:4000/v1"),
|
|
],
|
|
)
|
|
def test_normalize(raw, expected):
|
|
assert normalize_base_url(raw) == expected
|
|
|
|
|
|
@pytest.mark.parametrize("raw", ["ftp://host/v1", "not-a-url", "", " "])
|
|
def test_normalize_rejects(raw):
|
|
with pytest.raises(InvalidBaseURL):
|
|
normalize_base_url(raw)
|
|
|
|
|
|
@pytest.mark.parametrize(
|
|
"raw",
|
|
[
|
|
"https://user:secret@host:4000/v1",
|
|
"https://user@host:4000/v1",
|
|
"https://:secret@host:4000/v1",
|
|
],
|
|
)
|
|
def test_normalize_rejects_embedded_credentials(raw):
|
|
"""Authentication belongs in the API key field, never in base_url."""
|
|
with pytest.raises(CredentialsInBaseURL):
|
|
normalize_base_url(raw)
|
|
|
|
|
|
def test_credentials_in_base_url_is_an_invalid_base_url():
|
|
"""CredentialsInBaseURL must still be catchable as InvalidBaseURL."""
|
|
with pytest.raises(InvalidBaseURL):
|
|
normalize_base_url("https://user:secret@host:4000/v1")
|
|
|
|
|
|
def test_sanitize_strips_credentials():
|
|
assert (
|
|
sanitize_url_for_logging("https://user:secret@host:4000/v1")
|
|
== "https://***@host:4000/v1"
|
|
)
|
|
|
|
|
|
def test_sanitize_leaves_clean_urls_alone():
|
|
assert sanitize_url_for_logging("http://host:4000/v1") == "http://host:4000/v1"
|