fix: harden openai-compatible provider transitions

Clear provider-owned endpoint state when leaving openai-compatible configurations and reject base URLs with ambiguous or secret-bearing components.

Co-authored-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>

Signed-off-by: am <6e30cd56c30e030cd31bb0939b94a7c257c9a09d5ba2d92cf2735da45629f248@buzz.block.builderlab.xyz>
This commit is contained in:
am
2026-08-15 12:55:48 -07:00
parent b01d53546b
commit f2ea1b7fc9
11 changed files with 206 additions and 73 deletions
+28 -8
View File
@@ -905,7 +905,9 @@ impl Config {
parse_openai_api(env("OPENAI_COMPAT_API").as_deref())?,
),
Provider::OpenAiCompat => (
env("OPENAI_COMPAT_API_KEY").unwrap_or_default(),
env("OPENAI_COMPAT_API_KEY")
.map(|value| value.trim().to_string())
.unwrap_or_default(),
resolve_model(
buzz_agent_model.as_deref(),
env("OPENAI_COMPAT_MODEL").as_deref(),
@@ -1171,14 +1173,22 @@ fn resolve_provider(
}
fn parse_openai_compat_base_url(raw: Option<&str>) -> Result<String, String> {
const INVALID_URL: &str =
"config: OPENAI_COMPAT_BASE_URL must be an HTTP(S) URL without credentials, query, or fragment";
let value = raw
.map(str::trim)
.filter(|value| !value.is_empty())
.ok_or_else(|| "config: OPENAI_COMPAT_BASE_URL required for openai-compat".to_string())?;
let parsed = url::Url::parse(value)
.map_err(|_| "config: OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string())?;
if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() {
return Err("config: OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string());
let parsed = url::Url::parse(value).map_err(|_| INVALID_URL.to_string())?;
if !matches!(parsed.scheme(), "http" | "https")
|| parsed.host_str().is_none()
|| !parsed.username().is_empty()
|| parsed.password().is_some()
|| parsed.query().is_some()
|| parsed.fragment().is_some()
{
return Err(INVALID_URL.to_string());
}
Ok(value.trim_end_matches('/').to_string())
}
@@ -1493,9 +1503,19 @@ mod tests {
assert!(parse_openai_compat_base_url(None)
.unwrap_err()
.contains("required for openai-compat"));
assert!(parse_openai_compat_base_url(Some("ftp://localhost/v1"))
.unwrap_err()
.contains("valid HTTP(S) URL"));
for invalid in [
"ftp://localhost/v1",
"http://user:secret@localhost/v1",
"http://localhost/v1?tenant=x",
"http://localhost/v1#fragment",
] {
assert!(
parse_openai_compat_base_url(Some(invalid))
.unwrap_err()
.contains("without credentials, query, or fragment"),
"invalid={invalid}"
);
}
assert_eq!(
parse_openai_compat_base_url(Some(" http://localhost:11434/v1/// ")).unwrap(),
"http://localhost:11434/v1"
@@ -12,7 +12,7 @@ use super::managed_agent_definition::apply_model_provider_prompt_update;
use super::agent_models_env::env_value;
use super::agent_models_env::{
effective_discovery_provider, env_or_process_override, env_or_process_value,
redaction_env_with_value, DiscoveryProvider,
redaction_env_with_value, validate_openai_compat_base_url, DiscoveryProvider,
};
use super::agent_update_rollback::{rollback_failed_agent_update, AgentUpdateRollback};
@@ -367,19 +367,17 @@ fn openai_compatible_models_url_for_discovery(
provider: Option<&str>,
env: &BTreeMap<String, String>,
) -> Result<String, String> {
let is_compat =
provider.is_some_and(|value| value.trim().eq_ignore_ascii_case("openai-compat"));
let base_url = env_or_process_value(env, "OPENAI_COMPAT_BASE_URL");
let base_url = if provider.map(str::trim) == Some("openai-compat") {
let base_url = if is_compat {
base_url.ok_or_else(|| {
"OPENAI_COMPAT_BASE_URL required for OpenAI-compatible model discovery".to_string()
})?
} else {
base_url.unwrap_or_else(|| "https://api.openai.com/v1".to_string())
};
let parsed = url::Url::parse(base_url.trim())
.map_err(|_| "OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string())?;
if !matches!(parsed.scheme(), "http" | "https") || parsed.host_str().is_none() {
return Err("OPENAI_COMPAT_BASE_URL must be a valid HTTP(S) URL".to_string());
}
validate_openai_compat_base_url(&base_url)?;
Ok(format!("{}/models", base_url.trim().trim_end_matches('/')))
}
@@ -511,7 +509,9 @@ async fn discover_openai_compatible_models(
return Ok(None);
}
let is_compat = provider.as_deref().map(str::trim) == Some("openai-compat");
let is_compat = provider
.as_deref()
.is_some_and(|value| value.trim().eq_ignore_ascii_case("openai-compat"));
let api_key = if relay_mesh {
crate::managed_agents::RELAY_MESH_API_KEY_PLACEHOLDER.to_string()
} else if is_compat {
@@ -39,6 +39,22 @@ pub(super) fn env_or_process_override(env: &BTreeMap<String, String>, key: &str)
})
}
pub(super) fn validate_openai_compat_base_url(value: &str) -> Result<(), String> {
const INVALID_URL: &str =
"OPENAI_COMPAT_BASE_URL must be an HTTP(S) URL without credentials, query, or fragment";
let parsed = url::Url::parse(value.trim()).map_err(|_| INVALID_URL.to_string())?;
if !matches!(parsed.scheme(), "http" | "https")
|| parsed.host_str().is_none()
|| !parsed.username().is_empty()
|| parsed.password().is_some()
|| parsed.query().is_some()
|| parsed.fragment().is_some()
{
return Err(INVALID_URL.to_string());
}
Ok(())
}
/// Clone `env` with `key` set to the value a request actually used, so error
/// redaction masks the inherited process value and not just the mapped one.
pub(super) fn redaction_env_with_value(
@@ -126,3 +142,23 @@ pub(super) fn effective_discovery_provider(
inferred: true,
}
}
#[cfg(test)]
mod tests {
use super::validate_openai_compat_base_url;
#[test]
fn rejects_ambiguous_openai_compat_url_components() {
for value in [
"http://user:secret@localhost/v1",
"http://localhost/v1?tenant=x",
"http://localhost/v1#fragment",
] {
let error = validate_openai_compat_base_url(value).unwrap_err();
assert!(
error.contains("without credentials, query, or fragment"),
"value={value} error={error}"
);
}
}
}
@@ -460,8 +460,8 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec<Requirement> {
let provider = effective
.env
.get("BUZZ_AGENT_PROVIDER")
.filter(|v| !v.is_empty())
.map(String::as_str);
.map(|value| value.trim().to_ascii_lowercase())
.filter(|value| !value.is_empty());
if provider.is_none() {
missing.push(Requirement::NormalizedField {
field: "provider".to_string(),
@@ -475,7 +475,7 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec<Requirement> {
// databricks/databricks_v2, ANTHROPIC_MODEL for anthropic, etc.). The
// baked buzz-releases env sets DATABRICKS_MODEL but not BUZZ_AGENT_MODEL,
// so without this fallback agents baked from releases appear "not ready".
let provider_model_key = match provider {
let provider_model_key = match provider.as_deref() {
Some("databricks") | Some("databricks_v2") | Some("databricks-v2") => {
Some("DATABRICKS_MODEL")
}
@@ -503,7 +503,7 @@ fn buzz_agent_requirements(effective: &EffectiveAgentEnv) -> Vec<Requirement> {
// A key present with an empty value is treated as absent — matching the
// dialog's (envVars[key] ?? "").length === 0 emptiness check.
let env_key_missing = |key: &str| effective.env.get(key).is_none_or(|v| v.is_empty());
match provider {
match provider.as_deref() {
Some("anthropic")
if env_key_missing("ANTHROPIC_API_KEY") => {
missing.push(Requirement::EnvKey {
@@ -566,16 +566,17 @@ fn goose_requirements(
let provider = effective
.env
.get("GOOSE_PROVIDER")
.filter(|v| !v.is_empty())
.map(String::as_str);
.map(|value| value.trim().to_ascii_lowercase())
.filter(|value| !value.is_empty());
// Effective provider for credential checking: prefer env layer, then file.
let effective_provider = provider.or_else(|| {
file_cfg
.as_ref()
.and_then(|c| c.provider.as_deref())
.filter(|v| !v.is_empty())
});
let file_provider = file_cfg
.as_ref()
.and_then(|config| config.provider.as_deref())
.map(str::trim)
.filter(|value| !value.is_empty())
.map(str::to_ascii_lowercase);
let effective_provider = provider.as_deref().or(file_provider.as_deref());
if provider.is_none() {
// Silenced if the file config provides a provider.
@@ -783,7 +784,7 @@ mod tests {
let with_url = make_env(
"buzz-agent",
env_with(&[
("BUZZ_AGENT_PROVIDER", "openai-compat"),
("BUZZ_AGENT_PROVIDER", " OpenAI-Compat "),
("BUZZ_AGENT_MODEL", "llama3"),
("OPENAI_COMPAT_BASE_URL", "http://localhost:11434/v1"),
]),
+5 -1
View File
@@ -105,7 +105,11 @@ with a TypeScript lookup table or an id comparison in a component.
`onboarding-agent-defaults.spec.ts`.
9. **The defaults modal is progressively disclosed.** An unset global config
starts on the Buzz Agent-first deployment fallback and carries that visible
harness into the next saved edit. The `progressive-defaults` disclosure
harness into the next saved edit. Provider changes preserve typed API keys
in Advanced, but `OPENAI_COMPAT_BASE_URL` is provider-owned routing state and
must be cleared whenever a defaults/onboarding transition leaves
`openai-compat`; otherwise official OpenAI discovery inherits the stale
custom endpoint. The `progressive-defaults` disclosure
preset therefore begins at Provider for Buzz Agent, then reveals Model,
Effort, and Advanced only after a provider is configured. Harnesses whose
runtime metadata has no provider field skip that gate. Reveals animate their
@@ -37,7 +37,6 @@ import {
CARD_MINT_KEY_ANNOTATIONS,
CUSTOM_PROVIDER_DROPDOWN_VALUE,
getPersonaProviderOptions,
getProviderApiKeyEnvVar,
getProviderApiKeyLabel,
runtimeSupportsLlmProviderSelection,
} from "@/features/agents/ui/agentConfigOptions";
@@ -67,6 +66,7 @@ import { SettingsOptionGroup } from "@/features/settings/ui/SettingsOptionGroup"
import { AdvancedRequiredBadge } from "./AdvancedRequiredBadge";
import { CardMintKeyCue } from "./CardMintKeyCue";
import { getGlobalAgentCredentialState } from "./globalAgentCredentialState";
import { envVarsClearingOpenAiCompatBaseUrl } from "./providerEnvVarUpdates";
export const EMPTY_GLOBAL_CONFIG: GlobalAgentConfig = {
env_vars: {},
@@ -95,7 +95,9 @@ type AgentConfigDisclosure =
// - auto-select a valid model when the provider changes
// - keep the model select usable during discovery
// - preserve credential env vars across provider switches (the abandoned
// provider's key stays in env_vars — visible/deletable under Advanced)
// provider's key stays in env_vars — visible/deletable under Advanced), except
// provider-owned endpoint metadata is cleared when it would affect the next
// provider.
// - require a provider before model/effort are editable (no saveable
// invalid state — design principle #4)
const autoSelectModelOnProviderChange = true;
@@ -520,29 +522,23 @@ export function AgentConfigFields({
});
function handleProviderChange(value: string) {
userEditedProviderRef.current = true;
const previousApiKey = getProviderApiKeyEnvVar(effectiveProvider);
if (value === CUSTOM_PROVIDER_DROPDOWN_VALUE) {
const nextEnvVars = { ...config.env_vars };
if (!preserveCredentialEnvVarsOnProviderChange && previousApiKey) {
delete nextEnvVars[previousApiKey];
}
const nextEnvVars = envVarsClearingOpenAiCompatBaseUrl(
config.env_vars,
effectiveProvider,
"",
);
onIsCustomProviderChange(true);
onConfigChange({ ...config, env_vars: nextEnvVars, provider: null });
return;
}
const nextProvider =
value === AUTO_PROVIDER_DROPDOWN_VALUE || value === "" ? null : value;
const nextApiKey = getProviderApiKeyEnvVar(
const nextEnvVars = envVarsClearingOpenAiCompatBaseUrl(
config.env_vars,
effectiveProvider,
nextProvider ?? bakedProvider ?? "",
);
const nextEnvVars = { ...config.env_vars };
if (
!preserveCredentialEnvVarsOnProviderChange &&
previousApiKey &&
previousApiKey !== nextApiKey
) {
delete nextEnvVars[previousApiKey];
}
const providerChanged = nextProvider !== (config.provider ?? null);
onIsCustomProviderChange(false);
onConfigChange({
@@ -3,19 +3,23 @@ import test from "node:test";
import { openAiCompatibleBaseUrlError } from "./OpenAiCompatibleBaseUrlField.tsx";
test("OpenAI-compatible base URL accepts only absolute HTTP(S) URLs", () => {
test("OpenAI-compatible base URL accepts only safe absolute HTTP(S) URLs", () => {
const invalidMessage =
"Enter an HTTP or HTTPS URL without credentials, query, or fragment.";
assert.equal(openAiCompatibleBaseUrlError("http://localhost:11434/v1"), null);
assert.equal(
openAiCompatibleBaseUrlError(" https://models.example/v1/ "),
null,
);
assert.equal(openAiCompatibleBaseUrlError(""), "Base URL is required.");
assert.equal(
openAiCompatibleBaseUrlError("ftp://models.example/v1"),
"Enter a valid HTTP or HTTPS URL.",
);
assert.equal(
openAiCompatibleBaseUrlError("localhost:11434/v1"),
"Enter a valid HTTP or HTTPS URL.",
);
for (const value of [
"ftp://models.example/v1",
"localhost:11434/v1",
"https://user:secret@models.example/v1",
"https://models.example/v1?tenant=x",
"https://models.example/v1#fragment",
]) {
assert.equal(openAiCompatibleBaseUrlError(value), invalidMessage, value);
}
});
@@ -8,18 +8,26 @@ import {
export const OPENAI_COMPAT_BASE_URL = "OPENAI_COMPAT_BASE_URL";
function isSafeHttpBaseUrl(url: URL): boolean {
return (
(url.protocol === "http:" || url.protocol === "https:") &&
url.host.length > 0 &&
url.username.length === 0 &&
url.password.length === 0 &&
url.search.length === 0 &&
url.hash.length === 0
);
}
export function openAiCompatibleBaseUrlError(value: string): string | null {
const trimmed = value.trim();
if (trimmed.length === 0) return "Base URL is required.";
try {
const url = new URL(trimmed);
if ((url.protocol === "http:" || url.protocol === "https:") && url.host) {
return null;
}
if (isSafeHttpBaseUrl(new URL(trimmed))) return null;
} catch {
// Fall through to the actionable validation message.
}
return "Enter a valid HTTP or HTTPS URL.";
return "Enter an HTTP or HTTPS URL without credentials, query, or fragment.";
}
export function OpenAiCompatibleBaseUrlField({
@@ -3,6 +3,7 @@ import test from "node:test";
import {
envVarsClearingManagedApiKey,
envVarsClearingOpenAiCompatBaseUrl,
envVarsWithoutKey,
} from "./providerEnvVarUpdates.ts";
@@ -35,6 +36,45 @@ test("envVarsClearingManagedApiKey clears when leaving to a custom/empty provide
assert.deepEqual(next, {});
});
test("envVarsClearingOpenAiCompatBaseUrl clears only the compatible URL", () => {
const current = {
OPENAI_COMPAT_API_KEY: "sk-1",
OPENAI_COMPAT_BASE_URL: "http://localhost:11434/v1",
KEEP: "x",
};
assert.deepEqual(
envVarsClearingOpenAiCompatBaseUrl(current, " OpenAI-Compat ", "openai"),
{
OPENAI_COMPAT_API_KEY: "sk-1",
KEEP: "x",
},
);
assert.equal(
envVarsClearingOpenAiCompatBaseUrl(
current,
"openai-compat",
"OPENAI-COMPAT",
),
current,
);
});
test("envVarsClearingManagedApiKey clears the compatible URL when leaving compat", () => {
const next = envVarsClearingManagedApiKey(
{
OPENAI_COMPAT_API_KEY: "sk-1",
OPENAI_COMPAT_BASE_URL: "http://localhost:11434/v1",
KEEP: "x",
},
"openai-compat",
"openai",
);
assert.deepEqual(next, {
OPENAI_COMPAT_API_KEY: "sk-1",
KEEP: "x",
});
});
test("envVarsClearingManagedApiKey is a no-op when the managed key is shared or absent", () => {
const current = { ANTHROPIC_API_KEY: "sk-1" };
assert.equal(
@@ -1,6 +1,12 @@
import type { EnvVarsValue } from "./EnvVarsEditor";
import { getProviderApiKeyEnvVar } from "./agentConfigOptions";
const OPENAI_COMPAT_BASE_URL = "OPENAI_COMPAT_BASE_URL";
function normalizedProvider(provider: string): string {
return provider.trim().toLowerCase();
}
/**
* Pure env-var update helpers shared by the persona / create-agent /
* edit-agent dialogs. Every function returns the SAME reference when nothing
@@ -21,10 +27,24 @@ export function envVarsWithoutKey(
return next;
}
/** Remove the compatible endpoint when switching away from openai-compat. */
export function envVarsClearingOpenAiCompatBaseUrl(
current: EnvVarsValue,
previousProvider: string,
nextProvider: string,
): EnvVarsValue {
if (
normalizedProvider(previousProvider) === "openai-compat" &&
normalizedProvider(nextProvider) !== "openai-compat"
) {
return envVarsWithoutKey(current, OPENAI_COMPAT_BASE_URL);
}
return current;
}
/**
* Clear the previous provider's managed API key when switching providers.
* No-op when the previous provider has no managed key or the next provider
* uses the same one.
* Clear the previous provider's managed API key and provider-owned endpoint
* when switching providers.
*/
export function envVarsClearingManagedApiKey(
current: EnvVarsValue,
@@ -33,8 +53,14 @@ export function envVarsClearingManagedApiKey(
): EnvVarsValue {
const previousEnvVar = getProviderApiKeyEnvVar(previousProvider);
const nextEnvVar = getProviderApiKeyEnvVar(nextProvider);
let next = current;
if (previousEnvVar && previousEnvVar !== nextEnvVar) {
return envVarsWithoutKey(current, previousEnvVar);
next = envVarsWithoutKey(next, previousEnvVar);
}
return current;
next = envVarsClearingOpenAiCompatBaseUrl(
next,
previousProvider,
nextProvider,
);
return next;
}
@@ -8,10 +8,7 @@ import {
shouldClearKnownModelForSelectionScope,
} from "./agentConfigOptions";
import { shouldClearModelForRuntimeChange } from "./personaRuntimeModel";
import {
envVarsClearingManagedApiKey,
envVarsWithoutKey,
} from "./providerEnvVarUpdates";
import { envVarsClearingManagedApiKey } from "./providerEnvVarUpdates";
/**
* Pure transition functions for the runtime -> LLM provider -> model dropdown
@@ -92,10 +89,11 @@ export function selectionOnProviderDropdownChange(
const next = { ...current };
if (params.nextValue === CUSTOM_PROVIDER_DROPDOWN_VALUE) {
const previousEnvVar = getProviderApiKeyEnvVar(current.provider);
if (previousEnvVar) {
next.envVars = envVarsWithoutKey(next.envVars, previousEnvVar);
}
next.envVars = envVarsClearingManagedApiKey(
next.envVars,
current.provider,
"",
);
next.isCustomProviderEditing = true;
next.provider = "";
return next;