避免无效模型清单破坏 Codex 能力发现
自定义 API-key 上游可能将普通 OpenAI 模型列表作为 2xx 响应返回。现在只校验稳定 envelope,并把结构错误归类为可重试,以便冷缓存时换号且不污染缓存。 Constraint: Codex manifest schema 会随客户端演进,仅校验顶层 models 数组 Rejected: 在网关合成完整 manifest | 容易与上游 schema 漂移 Confidence: high Scope-risk: narrow Reversibility: clean Directive: 不要在此处校验 models 元素的易变字段 Tested: 后端 unit、integration、go vet Not-tested: 真实第三方 API-key 上游的在线 failover Co-authored-by: OmX <omx@oh-my-codex.dev>
This commit is contained in:
1 parent
a2779cd5f3
commit
43e95dfa45
3 files changed
+141
-5
No files matched your search
@@ -49,6 +49,7 @@ type codexModelsFailoverHTTPUpstream struct {
|
||||
accountIDs []int64
|
||||
firstErr error
|
||||
firstStatus int
|
||||
firstBody string
|
||||
statuses map[int64]int
|
||||
}
|
||||
|
||||
@@ -62,6 +63,14 @@ func (u *codexModelsFailoverHTTPUpstream) Do(_ *http.Request, _ string, accountI
|
||||
if u.firstErr != nil {
|
||||
return nil, u.firstErr
|
||||
}
|
||||
if u.firstBody != "" && !hasStatus {
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Status: "200 OK",
|
||||
Header: make(http.Header),
|
||||
Body: io.NopCloser(strings.NewReader(u.firstBody)),
|
||||
}, nil
|
||||
}
|
||||
if !hasStatus {
|
||||
status = u.firstStatus
|
||||
}
|
||||
@@ -150,6 +159,22 @@ func TestCodexModelsFailsOverFromUpstreamTransportError(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestCodexModelsFailsOverFromInvalidManifestEnvelope(t *testing.T) {
|
||||
handler, upstream, groupID := newCodexModelsFailoverTestHandler(http.StatusOK)
|
||||
upstream.firstBody = `{"object":"list","data":[]}`
|
||||
recorder := performCodexModelsRequest(t, handler, groupID)
|
||||
|
||||
if got, want := upstream.calls(), []int64{1, 2}; !equalInt64Slices(got, want) {
|
||||
t.Fatalf("upstream account calls: got %v, want %v", got, want)
|
||||
}
|
||||
if recorder.Code != http.StatusOK {
|
||||
t.Fatalf("status: got %d, want %d; body=%s", recorder.Code, http.StatusOK, recorder.Body.String())
|
||||
}
|
||||
if got, want := recorder.Body.String(), `{"models":[{"slug":"gpt-5.6-sol"}]}`; got != want {
|
||||
t.Fatalf("body: got %q, want %q", got, want)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCodexModelsDoesNotFailOverFromPermanentUpstreamStatus(t *testing.T) {
|
||||
statuses := []int{
|
||||
http.StatusBadRequest,
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
package service
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -219,10 +221,10 @@ func (c *codexModelsManifestCache) set(key string, manifest *CodexModelsManifest
|
||||
// FetchCodexModelsManifest fetches the live Codex models manifest from either
|
||||
// the ChatGPT backend for OAuth accounts or a custom upstream for API key accounts.
|
||||
//
|
||||
// The response body is passed through verbatim: the manifest schema evolves
|
||||
// with Codex client releases, and interpreting it here would force the gateway
|
||||
// to chase upstream changes. Passing it through keeps the gateway
|
||||
// schema-agnostic and always reflects the account's real entitlements.
|
||||
// After validating the stable top-level envelope, the response body is passed
|
||||
// through verbatim. Model entries evolve with Codex client releases, so the
|
||||
// gateway deliberately avoids interpreting their fields and reflects the
|
||||
// account's real entitlements without chasing upstream schema changes.
|
||||
func (s *OpenAIGatewayService) FetchCodexModelsManifest(ctx context.Context, account *Account, clientVersion, ifNoneMatch string) (*CodexModelsManifest, error) {
|
||||
if account == nil {
|
||||
return nil, infraerrors.New(http.StatusInternalServerError, "OPENAI_CODEX_MODELS_ACCOUNT_REQUIRED", "account is required")
|
||||
@@ -461,9 +463,43 @@ func (s *OpenAIGatewayService) fetchCodexModelsManifestUpstream(ctx context.Cont
|
||||
retryable: isRetryableCodexModelsManifestTransportError(err),
|
||||
}
|
||||
}
|
||||
if err := validateCodexModelsManifestEnvelope(body); err != nil {
|
||||
return nil, &codexModelsManifestUpstreamError{
|
||||
err: infraerrors.Newf(
|
||||
http.StatusBadGateway,
|
||||
"OPENAI_CODEX_MODELS_UPSTREAM_INVALID_MANIFEST",
|
||||
"codex models manifest upstream returned an invalid envelope: %v",
|
||||
err,
|
||||
),
|
||||
retryable: true,
|
||||
}
|
||||
}
|
||||
return &CodexModelsManifest{Body: body, ETag: resp.Header.Get("ETag")}, nil
|
||||
}
|
||||
|
||||
func validateCodexModelsManifestEnvelope(body []byte) error {
|
||||
var envelope map[string]json.RawMessage
|
||||
if err := json.Unmarshal(body, &envelope); err != nil {
|
||||
return fmt.Errorf("decode JSON object: %w", err)
|
||||
}
|
||||
if envelope == nil {
|
||||
return errors.New("expected a JSON object")
|
||||
}
|
||||
models, ok := envelope["models"]
|
||||
if !ok {
|
||||
return errors.New("missing top-level models array")
|
||||
}
|
||||
models = bytes.TrimSpace(models)
|
||||
var entries []json.RawMessage
|
||||
if len(models) == 0 || models[0] != '[' {
|
||||
return errors.New("top-level models field is not an array")
|
||||
}
|
||||
if err := json.Unmarshal(models, &entries); err != nil {
|
||||
return fmt.Errorf("decode top-level models array: %w", err)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func buildCodexModelsManifestCacheKey(request codexModelsManifestRequest) string {
|
||||
hasher := sha256.New()
|
||||
_, _ = fmt.Fprintf(hasher, "%d\n%d\n%s\n%s\n", request.accountID, request.credentialAccountID, request.proxyURL, request.url)
|
||||
|
||||
@@ -470,6 +470,81 @@ func TestFetchCodexModelsManifestAPIKeyCustomUpstream(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchCodexModelsManifestRejectsInvalidEnvelope(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
body string
|
||||
}{
|
||||
{name: "OpenAI models list", body: `{"object":"list","data":[]}`},
|
||||
{name: "invalid JSON", body: `{"models":`},
|
||||
{name: "non-object", body: `[]`},
|
||||
{name: "null object", body: `null`},
|
||||
{name: "missing models", body: `{}`},
|
||||
{name: "models object", body: `{"models":{}}`},
|
||||
{name: "models null", body: `{"models":null}`},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
upstream := &codexModelsHTTPUpstreamStub{do: func(_ *http.Request, _ string, _ int64, _ int) (*http.Response, error) {
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Header: make(http.Header),
|
||||
Body: io.NopCloser(strings.NewReader(tt.body)),
|
||||
}, nil
|
||||
}}
|
||||
|
||||
s := newCodexModelsAPIKeyTestService(upstream)
|
||||
_, err := s.FetchCodexModelsManifest(
|
||||
context.Background(),
|
||||
newCodexModelsAPIKeyTestAccount("https://upstream.example"),
|
||||
"0.144.0",
|
||||
"",
|
||||
)
|
||||
if err == nil {
|
||||
t.Fatal("expected invalid manifest error, got nil")
|
||||
}
|
||||
if infraerrors.Reason(err) != "OPENAI_CODEX_MODELS_UPSTREAM_INVALID_MANIFEST" {
|
||||
t.Errorf("error reason: got %q", infraerrors.Reason(err))
|
||||
}
|
||||
if !IsRetryableCodexModelsManifestError(err) {
|
||||
t.Error("invalid upstream manifest must be retryable")
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchCodexModelsManifestAPIKeyDoesNotCacheInvalidEnvelope(t *testing.T) {
|
||||
var calls atomic.Int32
|
||||
upstream := &codexModelsHTTPUpstreamStub{do: func(_ *http.Request, _ string, _ int64, _ int) (*http.Response, error) {
|
||||
body := `{"object":"list","data":[]}`
|
||||
if calls.Add(1) > 1 {
|
||||
body = `{"models":[{"slug":"gpt-5.6"}]}`
|
||||
}
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
Header: make(http.Header),
|
||||
Body: io.NopCloser(strings.NewReader(body)),
|
||||
}, nil
|
||||
}}
|
||||
|
||||
s := newCodexModelsAPIKeyTestService(upstream)
|
||||
account := newCodexModelsAPIKeyTestAccount("https://upstream.example")
|
||||
if _, err := s.FetchCodexModelsManifest(context.Background(), account, "0.144.0", ""); err == nil {
|
||||
t.Fatal("expected invalid manifest error on first fetch")
|
||||
}
|
||||
manifest, err := s.FetchCodexModelsManifest(context.Background(), account, "0.144.0", "")
|
||||
if err != nil {
|
||||
t.Fatalf("second fetch returned error: %v", err)
|
||||
}
|
||||
if got, want := string(manifest.Body), `{"models":[{"slug":"gpt-5.6"}]}`; got != want {
|
||||
t.Errorf("body: got %q, want %q", got, want)
|
||||
}
|
||||
if got := calls.Load(); got != 2 {
|
||||
t.Errorf("upstream calls: got %d, want 2", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestFetchCodexModelsManifestAPIKeySharedRefreshSurvivesCallerCancellation(t *testing.T) {
|
||||
const manifestBody = `{"models":[{"slug":"gpt-5.6"}]}`
|
||||
var calls atomic.Int32
|
||||
@@ -693,7 +768,7 @@ func TestFetchCodexModelsManifestAPIKeyCacheBoundsEntriesAndBodySize(t *testing.
|
||||
calls.Add(1)
|
||||
body := `{"models":[]}`
|
||||
if strings.Contains(req.URL.Host, "large") {
|
||||
body = strings.Repeat("x", (1<<20)+1)
|
||||
body = `{"models":[],"padding":"` + strings.Repeat("x", (1<<20)+1) + `"}`
|
||||
}
|
||||
return &http.Response{
|
||||
StatusCode: http.StatusOK,
|
||||
|
||||
Reference in new issue
Block a user