fix(install): decouple openai/ prefix strip from HERMES_CUSTOM_* auto-fill
The install.sh "OpenAI bridge" block bundled two distinct concerns under
one guard:
1. Auto-fill HERMES_CUSTOM_{BASE_URL,API_KEY,API_MODE} when operator
didn't set them
2. Strip the openai/ prefix from DEFAULT_MODEL (OpenAI rejects prefixed
model IDs with 400 "invalid model ID")
Both only fired when the operator had NOT pre-configured HERMES_CUSTOM_*.
That broke molecule-core#1987: the staging E2E now pins HERMES_CUSTOM_*
explicitly (to work around derive-provider.sh's #19 fix not reaching all
tenants). The pin skipped concern (1) intentionally — but also skipped
(2) unintentionally. Result: E2E routes to api.openai.com with the wrong
model name and hits 400.
Fix: separate the two concerns.
- (A) Auto-fill block keeps its original guard — runs only when operator
didn't configure.
- (B) New independent block: strip openai/ iff the FINAL
HERMES_CUSTOM_BASE_URL matches api.openai.com. Regex is anchored
(^https?://api\.openai\.com(/|$)) so lookalike domains
(api.openai.com.evil.internal, beta.api.openai.com) do NOT match.
Idempotent on already-bare model names.
Verified via scripts/test-install-prefix-strip.sh — 10 cases including:
A default bridge strips openai/ → gpt-4o
B operator-pinned OpenAI URL also strips → gpt-4o (#1987 path, was broken)
C vLLM URL keeps prefix → openai/my-finetune
D openrouter keeps prefix → openai/gpt-4o
E minimax untouched → minimax/MiniMax-M2.7
G lookalike domain NOT stripped → openai/gpt-4o (anti-spoofing)
H http://api.openai.com also strips → gpt-4o
I subdomain beta.api.openai.com NOT strip → openai/gpt-4o
All 10 pass. Plus a parity check greps install.sh to ensure the inlined
logic matches what ships.
No behavioral change for any existing working config (scenarios A, C, D,
E, F above — all unchanged). Only fixes the broken scenario B.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Hongming Wang
parent
1ef1f53b2f
commit
dbf86a142e
47
install.sh
47
install.sh
@ -162,30 +162,45 @@ HERMES_DEFAULT_MODEL="${DEFAULT_MODEL}" \
|
||||
# the authoritative list. Therefore the OpenAI bridge uses `custom`
|
||||
# provider pointed at api.openai.com.
|
||||
#
|
||||
# Three things have to line up for this to actually work:
|
||||
# Two independent concerns:
|
||||
#
|
||||
# (1) Keep PROVIDER=custom when only OPENAI_API_KEY is set. Setting
|
||||
# PROVIDER=openai crashes the gateway with "Unknown provider".
|
||||
# (A) Auto-fill HERMES_CUSTOM_{BASE_URL,API_KEY,API_MODE} when the
|
||||
# operator hasn't — so the common case (just OPENAI_API_KEY) Just
|
||||
# Works. Operators who configure HERMES_CUSTOM_* themselves (vLLM,
|
||||
# LM Studio, a custom OpenAI-compat gateway) skip this step and
|
||||
# keep their own values.
|
||||
#
|
||||
# (2) Strip the `openai/` prefix from DEFAULT_MODEL. Custom passes
|
||||
# the model name verbatim to the endpoint; OpenAI rejects
|
||||
# `openai/gpt-4o` with 400 model_not_found (expects `gpt-4o`).
|
||||
# (B) Strip the `openai/` provider prefix from DEFAULT_MODEL when the
|
||||
# final request target is api.openai.com, regardless of who set
|
||||
# HERMES_CUSTOM_BASE_URL. OpenAI rejects `openai/gpt-4o` with 400
|
||||
# "invalid model ID" — it expects bare `gpt-4o`. This concern is
|
||||
# independent of who configured the routing: if requests land at
|
||||
# api.openai.com, the prefix must go.
|
||||
#
|
||||
# (3) Set api_mode: "chat_completions" in the emitted config.yaml.
|
||||
# Without this, hermes defaults custom provider to codex_responses
|
||||
# which hits /v1/responses + sends include=[reasoning.encrypted_content].
|
||||
# gpt-4o / gpt-4.1 reject that with 400 (only o1 family supports it).
|
||||
#
|
||||
# Fires ONLY when operator hasn't configured HERMES_CUSTOM_* themselves —
|
||||
# explicit custom endpoints (vLLM, LM Studio) skip the prefix strip and
|
||||
# chat_completions default.
|
||||
# These were bundled in a single guard before 2026-04-24, which caused:
|
||||
# operators who pinned HERMES_CUSTOM_{BASE_URL,API_KEY,API_MODE} to
|
||||
# api.openai.com (e.g. the molecule-core E2E test after PR #1987) got
|
||||
# the right routing but not the prefix strip, and hit OpenAI 400.
|
||||
# Separating (A) and (B) makes both paths correct.
|
||||
|
||||
# (A) auto-fill defaults when operator hasn't configured custom
|
||||
if [ "${PROVIDER}" = "custom" ] && [ -n "${OPENAI_API_KEY:-}" ] && [ -z "${HERMES_CUSTOM_BASE_URL:-}" ] && [ -z "${HERMES_CUSTOM_API_KEY:-}" ]; then
|
||||
export HERMES_CUSTOM_BASE_URL="https://api.openai.com/v1"
|
||||
export HERMES_CUSTOM_API_KEY="${OPENAI_API_KEY}"
|
||||
export HERMES_CUSTOM_API_MODE="chat_completions"
|
||||
# Strip the `openai/` provider prefix — OpenAI expects bare model names.
|
||||
echo "[install.sh] bridged OPENAI_API_KEY → custom provider @ api.openai.com (api_mode=chat_completions)"
|
||||
fi
|
||||
|
||||
# (B) strip the openai/ prefix ONLY when the final URL is api.openai.com.
|
||||
# The regex intentionally anchors to start + requires a `/` or end-of-string
|
||||
# after `api.openai.com` so lookalike domains (api.openai.com.evil.internal)
|
||||
# do not match. Idempotent when there's no prefix.
|
||||
if [[ "${HERMES_CUSTOM_BASE_URL:-}" =~ ^https?://api\.openai\.com(/|$) ]]; then
|
||||
BEFORE="${DEFAULT_MODEL}"
|
||||
DEFAULT_MODEL="${DEFAULT_MODEL#openai/}"
|
||||
echo "[install.sh] bridged OPENAI_API_KEY → custom provider @ api.openai.com (api_mode=chat_completions, model=${DEFAULT_MODEL})"
|
||||
if [ "${BEFORE}" != "${DEFAULT_MODEL}" ]; then
|
||||
echo "[install.sh] stripped openai/ prefix → model=${DEFAULT_MODEL} (routing to api.openai.com)"
|
||||
fi
|
||||
fi
|
||||
|
||||
{
|
||||
|
||||
172
scripts/test-install-prefix-strip.sh
Executable file
172
scripts/test-install-prefix-strip.sh
Executable file
@ -0,0 +1,172 @@
|
||||
#!/usr/bin/env bash
|
||||
# test-install-prefix-strip.sh — regression tests for the step-(B) openai/
|
||||
# prefix strip in install.sh.
|
||||
#
|
||||
# install.sh's prefix strip used to be coupled to step (A)'s auto-fill
|
||||
# guard: if the operator pre-configured HERMES_CUSTOM_{BASE_URL,API_KEY,
|
||||
# API_MODE}, step (A) skipped, which also skipped the prefix strip.
|
||||
# That broke molecule-core#1987 (staging E2E) which pins HERMES_CUSTOM_*
|
||||
# to bypass derive-provider.sh flakiness.
|
||||
#
|
||||
# This test pins the decoupled behavior: strip when the final URL is
|
||||
# api.openai.com, keep the prefix otherwise.
|
||||
#
|
||||
# Design: rather than partial-source install.sh (which boots hermes,
|
||||
# installs apt packages, etc.), we inline the exact two blocks here and
|
||||
# a `verify-parity` step greps install.sh to ensure the inlined logic
|
||||
# matches what ships.
|
||||
|
||||
set -u
|
||||
HERE="$(cd "$(dirname "$0")" && pwd)"
|
||||
INSTALL="$HERE/../install.sh"
|
||||
|
||||
PASS=0
|
||||
FAIL=0
|
||||
|
||||
# The logic under test — mirrored from install.sh. If install.sh changes,
|
||||
# either keep this in sync or make the test fail the parity check below.
|
||||
apply_install_logic() {
|
||||
# (A) auto-fill defaults when operator hasn't configured custom
|
||||
if [ "${PROVIDER:-}" = "custom" ] \
|
||||
&& [ -n "${OPENAI_API_KEY:-}" ] \
|
||||
&& [ -z "${HERMES_CUSTOM_BASE_URL:-}" ] \
|
||||
&& [ -z "${HERMES_CUSTOM_API_KEY:-}" ]; then
|
||||
export HERMES_CUSTOM_BASE_URL="https://api.openai.com/v1"
|
||||
export HERMES_CUSTOM_API_KEY="${OPENAI_API_KEY}"
|
||||
export HERMES_CUSTOM_API_MODE="chat_completions"
|
||||
fi
|
||||
|
||||
# (B) strip openai/ prefix iff final URL is api.openai.com (decoupled from A)
|
||||
if [[ "${HERMES_CUSTOM_BASE_URL:-}" =~ ^https?://api\.openai\.com(/|$) ]]; then
|
||||
DEFAULT_MODEL="${DEFAULT_MODEL#openai/}"
|
||||
fi
|
||||
}
|
||||
|
||||
assert_model() {
|
||||
local label="$1" expected="$2"
|
||||
shift 2
|
||||
local actual
|
||||
actual=$(bash -c "
|
||||
set -u
|
||||
$(declare -f apply_install_logic)
|
||||
PROVIDER=''
|
||||
OPENAI_API_KEY=''
|
||||
OPENROUTER_API_KEY=''
|
||||
MINIMAX_API_KEY=''
|
||||
HERMES_CUSTOM_BASE_URL=''
|
||||
HERMES_CUSTOM_API_KEY=''
|
||||
HERMES_CUSTOM_API_MODE=''
|
||||
DEFAULT_MODEL=''
|
||||
$*
|
||||
apply_install_logic
|
||||
printf '%s' \"\$DEFAULT_MODEL\"
|
||||
" 2>/dev/null)
|
||||
if [ "$actual" = "$expected" ]; then
|
||||
echo " PASS $label → $actual"
|
||||
PASS=$((PASS+1))
|
||||
else
|
||||
echo " FAIL $label → got '$actual', expected '$expected'"
|
||||
FAIL=$((FAIL+1))
|
||||
fi
|
||||
}
|
||||
|
||||
echo "== install.sh prefix-strip =="
|
||||
|
||||
# --- Case A: default bridge path (no operator HERMES_CUSTOM_*) ---
|
||||
assert_model "A: default bridge strips openai/" "gpt-4o" '
|
||||
PROVIDER=custom
|
||||
OPENAI_API_KEY=sk-test
|
||||
DEFAULT_MODEL=openai/gpt-4o
|
||||
'
|
||||
|
||||
# --- Case B: operator-configured HERMES_CUSTOM_* → OpenAI (the #1987 path) ---
|
||||
assert_model "B: operator-pinned OpenAI URL strips openai/ (#1987)" "gpt-4o" '
|
||||
PROVIDER=custom
|
||||
OPENAI_API_KEY=sk-test
|
||||
HERMES_CUSTOM_BASE_URL=https://api.openai.com/v1
|
||||
HERMES_CUSTOM_API_KEY=sk-test
|
||||
HERMES_CUSTOM_API_MODE=chat_completions
|
||||
DEFAULT_MODEL=openai/gpt-4o
|
||||
'
|
||||
|
||||
# --- Case C: operator-configured HERMES_CUSTOM_* → vLLM/local server → NO strip ---
|
||||
assert_model "C: vLLM URL keeps prefix (user namespace)" "openai/my-finetune" '
|
||||
PROVIDER=custom
|
||||
OPENAI_API_KEY=sk-test
|
||||
HERMES_CUSTOM_BASE_URL=http://localhost:8000/v1
|
||||
HERMES_CUSTOM_API_KEY=none
|
||||
DEFAULT_MODEL=openai/my-finetune
|
||||
'
|
||||
|
||||
# --- Case D: PROVIDER=openrouter → no strip (OR expects prefix) ---
|
||||
assert_model "D: openrouter keeps prefix" "openai/gpt-4o" '
|
||||
PROVIDER=openrouter
|
||||
OPENROUTER_API_KEY=sk-or-test
|
||||
DEFAULT_MODEL=openai/gpt-4o
|
||||
'
|
||||
|
||||
# --- Case E: PROVIDER=minimax, model has different prefix → no strip ---
|
||||
assert_model "E: minimax model untouched" "minimax/MiniMax-M2.7" '
|
||||
PROVIDER=minimax
|
||||
MINIMAX_API_KEY=test
|
||||
DEFAULT_MODEL=minimax/MiniMax-M2.7
|
||||
'
|
||||
|
||||
# --- Case F: OpenAI URL but model already bare (idempotent) ---
|
||||
assert_model "F: idempotent on already-bare model" "gpt-4o" '
|
||||
PROVIDER=custom
|
||||
OPENAI_API_KEY=sk-test
|
||||
HERMES_CUSTOM_BASE_URL=https://api.openai.com/v1
|
||||
HERMES_CUSTOM_API_KEY=sk-test
|
||||
DEFAULT_MODEL=gpt-4o
|
||||
'
|
||||
|
||||
# --- Case G: lookalike domain must NOT match ---
|
||||
assert_model "G: lookalike domain api.openai.com.evil.internal NOT stripped" "openai/gpt-4o" '
|
||||
PROVIDER=custom
|
||||
HERMES_CUSTOM_BASE_URL=https://api.openai.com.evil.internal/v1
|
||||
HERMES_CUSTOM_API_KEY=stolen
|
||||
DEFAULT_MODEL=openai/gpt-4o
|
||||
'
|
||||
|
||||
# --- Case H: http:// (not https) also matches (for local proxy fronting OpenAI) ---
|
||||
assert_model "H: http:// api.openai.com still strips" "gpt-4o" '
|
||||
PROVIDER=custom
|
||||
HERMES_CUSTOM_BASE_URL=http://api.openai.com/v1
|
||||
HERMES_CUSTOM_API_KEY=sk-test
|
||||
DEFAULT_MODEL=openai/gpt-4o
|
||||
'
|
||||
|
||||
# --- Case I: subdomain of api.openai.com (unlikely) must NOT match ---
|
||||
assert_model "I: beta.api.openai.com NOT stripped" "openai/gpt-4o" '
|
||||
PROVIDER=custom
|
||||
HERMES_CUSTOM_BASE_URL=https://beta.api.openai.com/v1
|
||||
HERMES_CUSTOM_API_KEY=sk-test
|
||||
DEFAULT_MODEL=openai/gpt-4o
|
||||
'
|
||||
|
||||
# --- Parity check: install.sh must contain the exact logic we inlined here ---
|
||||
# Uses grep -F (fixed string) to avoid regex escaping hell. Each pattern is
|
||||
# a short unique substring from the real install.sh block.
|
||||
echo
|
||||
echo "== parity with install.sh =="
|
||||
PARITY_FAIL=0
|
||||
for pattern in \
|
||||
'[ "${PROVIDER}" = "custom" ] && [ -n "${OPENAI_API_KEY:-}" ] && [ -z "${HERMES_CUSTOM_BASE_URL:-}" ] && [ -z "${HERMES_CUSTOM_API_KEY:-}" ]' \
|
||||
'=~ ^https?://api\.openai\.com(/|$)' \
|
||||
'DEFAULT_MODEL="${DEFAULT_MODEL#openai/}"'; do
|
||||
if ! grep -F -q -- "$pattern" "$INSTALL"; then
|
||||
echo " FAIL install.sh missing substring: $pattern"
|
||||
PARITY_FAIL=$((PARITY_FAIL+1))
|
||||
fi
|
||||
done
|
||||
if [ "$PARITY_FAIL" -eq 0 ]; then
|
||||
echo " PASS install.sh contains expected logic blocks"
|
||||
PASS=$((PASS+1))
|
||||
else
|
||||
FAIL=$((FAIL+PARITY_FAIL))
|
||||
fi
|
||||
|
||||
echo
|
||||
echo "== results: $PASS passed, $FAIL failed =="
|
||||
[ "$FAIL" -eq 0 ]
|
||||
Loading…
Reference in New Issue
Block a user