mirror of
https://github.com/router-for-me/CLIProxyAPIPlus.git
synced 2026-03-08 06:43:41 +00:00
fix: address PR review feedback
- Fix SSRF: validate API endpoint host against allowlist before use - Limit /models response body to 2MB to prevent memory exhaustion (DoS) - Use MakeAuthenticatedRequest for consistent headers across API calls - Trim trailing slash on API endpoint to prevent double-slash URLs - Use ListModelsWithGitHubToken to simplify token exchange + listing - Deduplicate model IDs to prevent incorrect registry reference counting - Remove dead capabilities enrichment code block - Remove unused ModelExtra field with misleading json:"-" tag - Extract magic numbers to named constants (defaultCopilotContextLength) - Remove redundant hyphenID == id check (already filtered by Contains) - Use defer cancel() for context timeout in service.go
This commit is contained in:
@@ -8,6 +8,8 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"strings"
|
||||
"time"
|
||||
|
||||
"github.com/router-for-me/CLIProxyAPI/v6/internal/config"
|
||||
@@ -224,14 +226,13 @@ func (c *CopilotAuth) MakeAuthenticatedRequest(ctx context.Context, method, url
|
||||
|
||||
// CopilotModelEntry represents a single model entry returned by the Copilot /models API.
|
||||
type CopilotModelEntry struct {
|
||||
ID string `json:"id"`
|
||||
Object string `json:"object"`
|
||||
Created int64 `json:"created"`
|
||||
OwnedBy string `json:"owned_by"`
|
||||
Name string `json:"name,omitempty"`
|
||||
Version string `json:"version,omitempty"`
|
||||
Capabilities map[string]any `json:"capabilities,omitempty"`
|
||||
ModelExtra map[string]any `json:"-"` // catch-all for unknown fields
|
||||
ID string `json:"id"`
|
||||
Object string `json:"object"`
|
||||
Created int64 `json:"created"`
|
||||
OwnedBy string `json:"owned_by"`
|
||||
Name string `json:"name,omitempty"`
|
||||
Version string `json:"version,omitempty"`
|
||||
Capabilities map[string]any `json:"capabilities,omitempty"`
|
||||
}
|
||||
|
||||
// CopilotModelsResponse represents the response from the Copilot /models endpoint.
|
||||
@@ -240,6 +241,17 @@ type CopilotModelsResponse struct {
|
||||
Object string `json:"object"`
|
||||
}
|
||||
|
||||
// maxModelsResponseSize is the maximum allowed response size from the /models endpoint (2 MB).
|
||||
const maxModelsResponseSize = 2 * 1024 * 1024
|
||||
|
||||
// allowedCopilotAPIHosts is the set of hosts that are considered safe for Copilot API requests.
|
||||
var allowedCopilotAPIHosts = map[string]bool{
|
||||
"api.githubcopilot.com": true,
|
||||
"api.individual.githubcopilot.com": true,
|
||||
"api.business.githubcopilot.com": true,
|
||||
"copilot-proxy.githubusercontent.com": true,
|
||||
}
|
||||
|
||||
// ListModels fetches the list of available models from the Copilot API.
|
||||
// It requires a valid Copilot API token (not the GitHub access token).
|
||||
func (c *CopilotAuth) ListModels(ctx context.Context, apiToken *CopilotAPIToken) ([]CopilotModelEntry, error) {
|
||||
@@ -247,23 +259,22 @@ func (c *CopilotAuth) ListModels(ctx context.Context, apiToken *CopilotAPIToken)
|
||||
return nil, fmt.Errorf("copilot: api token is required for listing models")
|
||||
}
|
||||
|
||||
// Build models URL, validating the endpoint host to prevent SSRF.
|
||||
modelsURL := copilotAPIEndpoint + "/models"
|
||||
if apiToken.Endpoints.API != "" {
|
||||
modelsURL = apiToken.Endpoints.API + "/models"
|
||||
if ep := strings.TrimRight(apiToken.Endpoints.API, "/"); ep != "" {
|
||||
parsed, err := url.Parse(ep)
|
||||
if err == nil && parsed.Scheme == "https" && allowedCopilotAPIHosts[parsed.Host] {
|
||||
modelsURL = ep + "/models"
|
||||
} else {
|
||||
log.Warnf("copilot: ignoring untrusted API endpoint %q, using default", ep)
|
||||
}
|
||||
}
|
||||
|
||||
req, err := http.NewRequestWithContext(ctx, http.MethodGet, modelsURL, nil)
|
||||
req, err := c.MakeAuthenticatedRequest(ctx, http.MethodGet, modelsURL, nil, apiToken)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("copilot: failed to create models request: %w", err)
|
||||
}
|
||||
|
||||
req.Header.Set("Authorization", "Bearer "+apiToken.Token)
|
||||
req.Header.Set("Accept", "application/json")
|
||||
req.Header.Set("User-Agent", copilotUserAgent)
|
||||
req.Header.Set("Editor-Version", copilotEditorVersion)
|
||||
req.Header.Set("Editor-Plugin-Version", copilotPluginVersion)
|
||||
req.Header.Set("Copilot-Integration-Id", copilotIntegrationID)
|
||||
|
||||
resp, err := c.httpClient.Do(req)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("copilot: models request failed: %w", err)
|
||||
@@ -274,7 +285,9 @@ func (c *CopilotAuth) ListModels(ctx context.Context, apiToken *CopilotAPIToken)
|
||||
}
|
||||
}()
|
||||
|
||||
bodyBytes, err := io.ReadAll(resp.Body)
|
||||
// Limit response body to prevent memory exhaustion.
|
||||
limitedReader := io.LimitReader(resp.Body, maxModelsResponseSize)
|
||||
bodyBytes, err := io.ReadAll(limitedReader)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("copilot: failed to read models response: %w", err)
|
||||
}
|
||||
|
||||
@@ -50,9 +50,6 @@ func GitHubCopilotAliasesFromModels(modelIDs []string) []OAuthModelAlias {
|
||||
continue
|
||||
}
|
||||
hyphenID := strings.ReplaceAll(id, ".", "-")
|
||||
if hyphenID == id {
|
||||
continue
|
||||
}
|
||||
key := id + "→" + hyphenID
|
||||
if _, ok := seen[key]; ok {
|
||||
continue
|
||||
|
||||
@@ -1266,6 +1266,13 @@ func isHTTPSuccess(statusCode int) bool {
|
||||
return statusCode >= 200 && statusCode < 300
|
||||
}
|
||||
|
||||
const (
|
||||
// defaultCopilotContextLength is the default context window for unknown Copilot models.
|
||||
defaultCopilotContextLength = 128000
|
||||
// defaultCopilotMaxCompletionTokens is the default max output tokens for unknown Copilot models.
|
||||
defaultCopilotMaxCompletionTokens = 16384
|
||||
)
|
||||
|
||||
// FetchGitHubCopilotModels dynamically fetches available models from the GitHub Copilot API.
|
||||
// It exchanges the GitHub access token stored in auth.Metadata for a Copilot API token,
|
||||
// then queries the /models endpoint. Falls back to the static registry on any failure.
|
||||
@@ -1283,13 +1290,7 @@ func FetchGitHubCopilotModels(ctx context.Context, auth *cliproxyauth.Auth, cfg
|
||||
|
||||
copilotAuth := copilotauth.NewCopilotAuth(cfg)
|
||||
|
||||
apiToken, err := copilotAuth.GetCopilotAPIToken(ctx, accessToken)
|
||||
if err != nil {
|
||||
log.Warnf("github-copilot: failed to get API token for model listing: %v, using static models", err)
|
||||
return registry.GetGitHubCopilotModels()
|
||||
}
|
||||
|
||||
entries, err := copilotAuth.ListModels(ctx, apiToken)
|
||||
entries, err := copilotAuth.ListModelsWithGitHubToken(ctx, accessToken)
|
||||
if err != nil {
|
||||
log.Warnf("github-copilot: failed to fetch dynamic models: %v, using static models", err)
|
||||
return registry.GetGitHubCopilotModels()
|
||||
@@ -1309,10 +1310,16 @@ func FetchGitHubCopilotModels(ctx context.Context, auth *cliproxyauth.Auth, cfg
|
||||
|
||||
now := time.Now().Unix()
|
||||
models := make([]*registry.ModelInfo, 0, len(entries))
|
||||
seen := make(map[string]struct{}, len(entries))
|
||||
for _, entry := range entries {
|
||||
if entry.ID == "" {
|
||||
continue
|
||||
}
|
||||
// Deduplicate model IDs to avoid incorrect reference counting.
|
||||
if _, dup := seen[entry.ID]; dup {
|
||||
continue
|
||||
}
|
||||
seen[entry.ID] = struct{}{}
|
||||
|
||||
m := ®istry.ModelInfo{
|
||||
ID: entry.ID,
|
||||
@@ -1331,11 +1338,6 @@ func FetchGitHubCopilotModels(ctx context.Context, auth *cliproxyauth.Auth, cfg
|
||||
m.DisplayName = entry.ID
|
||||
}
|
||||
|
||||
// Enrich from capabilities if available
|
||||
if caps, ok := entry.Capabilities["type"].(string); ok && caps != "" {
|
||||
_ = caps // reserved for future use
|
||||
}
|
||||
|
||||
// Merge known metadata from the static fallback list
|
||||
if static, ok := staticMap[entry.ID]; ok {
|
||||
if m.DisplayName == entry.ID && static.DisplayName != "" {
|
||||
@@ -1349,8 +1351,8 @@ func FetchGitHubCopilotModels(ctx context.Context, auth *cliproxyauth.Auth, cfg
|
||||
} else {
|
||||
// Sensible defaults for models not in the static list
|
||||
m.Description = entry.ID + " via GitHub Copilot"
|
||||
m.ContextLength = 128000
|
||||
m.MaxCompletionTokens = 16384
|
||||
m.ContextLength = defaultCopilotContextLength
|
||||
m.MaxCompletionTokens = defaultCopilotMaxCompletionTokens
|
||||
}
|
||||
|
||||
models = append(models, m)
|
||||
|
||||
@@ -867,8 +867,8 @@ func (s *Service) registerModelsForAuth(a *coreauth.Auth) {
|
||||
models = applyExcludedModels(models, excluded)
|
||||
case "github-copilot":
|
||||
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
|
||||
defer cancel()
|
||||
models = executor.FetchGitHubCopilotModels(ctx, a, s.cfg)
|
||||
cancel()
|
||||
models = applyExcludedModels(models, excluded)
|
||||
case "kiro":
|
||||
models = s.fetchKiroModels(a)
|
||||
|
||||
Reference in New Issue
Block a user