feats:replace GLM-4v authentication headers to support customize api key - #1536
Conversation
WalkthroughThe changes remove the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Suggested reviewers
Poem
Note 🔌 MCP (Model Context Protocol) integration is now available in Early Access!Pro users can now connect to remote MCP servers under the Integrations page to get reviews and chat conversations that understand additional development context. ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
relay/channel/zhipu_4v/adaptor.go(1 hunks)relay/channel/zhipu_4v/relay-zhipu_v4.go(0 hunks)
💤 Files with no reviewable changes (1)
- relay/channel/zhipu_4v/relay-zhipu_v4.go
🔇 Additional comments (1)
relay/channel/zhipu_4v/adaptor.go (1)
57-57: Auth header change aligns with BigModel HTTP auth (Bearer API key).Directly setting
Authorization: Bearer <apiKey>is consistent with the provider’s HTTP docs and removes unnecessary JWT complexity. Good simplification.
| channel.SetupApiRequestHeader(info, c, req) | ||
| token := getZhipuToken(info.ApiKey) | ||
| req.Set("Authorization", token) | ||
| req.Set("Authorization", "Bearer "+info.ApiKey) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Harden header construction: trim, handle pre-prefixed keys, and empty key guard.
Minor robustness improvements:
- Return an explicit error if
info.ApiKeyis empty to avoid a guaranteed 401. - Trim whitespace and avoid double “Bearer ” when the configured key is already prefixed.
Apply this diff within SetupRequestHeader:
- req.Set("Authorization", "Bearer "+info.ApiKey)
+ if strings.TrimSpace(info.ApiKey) == "" {
+ return errors.New("missing API key for Zhipu channel")
+ }
+ ak := strings.TrimSpace(info.ApiKey)
+ if strings.HasPrefix(strings.ToLower(ak), "bearer ") {
+ req.Set("Authorization", ak)
+ } else {
+ req.Set("Authorization", "Bearer "+ak)
+ }And add the import:
import "strings"Verification (ensure no stale JWT usage remains and check for already-prefixed keys):
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find any references to the removed JWT helper to prevent build-time surprises.
rg -n --hidden --glob '!*dist*' --glob '!*build*' 'getZhipuToken|jwt' || echo "No references to getZhipuToken/jwt found."
# Spot other Authorization constructions for consistency.
rg -n 'Authorization"\s*,\s*"Bearer\s*\+.*ApiKey' -A2 -B2 || true
# Find places where API keys may already include 'Bearer ' to gauge risk.
rg -n 'ApiKey.*Bearer' -A2 -B2 || trueLength of output: 2387
Let's locate and inspect SetupRequestHeader in the zhipu_4v adapter to verify if the trimming/prefix patch has already been applied:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the zhipu_4v adaptor file
echo "Searching for zhipu_4v/adaptor.go..."
find relay/channel -type f | grep 'zhipu_4v/adaptor.go' || true
# Show the SetupRequestHeader implementation around line 57
echo "Extracting SetupRequestHeader definition from zhipu_4v/adaptor.go..."
rg -n -A5 -B5 'func SetupRequestHeader' relay/channel/zhipu_4v/adaptor.go || trueLength of output: 422
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Search for occurrences of info.ApiKey to locate the header setup
echo "Searching for info.ApiKey usages..."
rg -n 'info\.ApiKey' -A5 -B5Length of output: 28759
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display the package declaration and imports for relay/channel/zhipu_4v/adaptor.go
sed -n '1,60p' relay/channel/zhipu_4v/adaptor.goLength of output: 1700
Improve Zhipu 4v Authorization header handling
Please harden SetupRequestHeader in relay/channel/zhipu_4v/adaptor.go to guard against empty or already-prefixed API keys:
• Return an explicit error when info.ApiKey is blank.
• Trim surrounding whitespace.
• Detect and avoid double-“Bearer ” prefixes.
Apply this patch:
diff --git a/relay/channel/zhipu_4v/adaptor.go b/relay/channel/zhipu_4v/adaptor.go
index abc1234..def5678 100644
--- a/relay/channel/zhipu_4v/adaptor.go
+++ b/relay/channel/zhipu_4v/adaptor.go
@@ -1,7 +1,8 @@
import (
"errors"
+ "strings"
"fmt"
"io"
"net/http"
@@ -55,7 +56,16 @@ func (a *Adaptor) SetupRequestHeader(c *gin.Context, req *http.Header, info *relaycommon.RelayInfo) error {
channel.SetupApiRequestHeader(info, c, req)
- req.Set("Authorization", "Bearer "+info.ApiKey)
+ // Guard empty key
+ if strings.TrimSpace(info.ApiKey) == "" {
+ return errors.New("missing API key for Zhipu channel")
+ }
+ // Trim and handle existing Bearer prefix
+ ak := strings.TrimSpace(info.ApiKey)
+ if strings.HasPrefix(strings.ToLower(ak), "bearer ") {
+ req.Set("Authorization", ak)
+ } else {
+ req.Set("Authorization", "Bearer "+ak)
+ }
return nil
}This ensures consistent behavior and clearer errors when the key is invalid.
🤖 Prompt for AI Agents
In relay/channel/zhipu_4v/adaptor.go at line 57, improve the SetupRequestHeader
function by first checking if info.ApiKey is empty or only whitespace and return
an explicit error if so. Then trim any surrounding whitespace from info.ApiKey.
Before setting the Authorization header, detect if the key already starts with
"Bearer " to avoid double prefixing. If it does not, prepend "Bearer " to the
trimmed key. This ensures the header is set consistently and errors are clear
when the API key is invalid.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
.github/workflows/docker-image-alpha.yml (1)
43-50: Ensure reliable multi-arch builds and faster CI: add QEMU and Buildx cache.
- Without QEMU, cross-building arm64 on amd64 runners can fail or be slow.
- GHA cache drastically speeds up rebuilds.
Add QEMU setup (insert after “Set up Docker Buildx” step):
- name: Set up QEMU uses: docker/setup-qemu-action@v3Augment the build step with cache:
- name: Build and push Docker image uses: docker/build-push-action@v5 with: context: . platforms: linux/amd64,linux/arm64 push: true tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} + cache-from: type=gha + cache-to: type=gha,mode=max
🧹 Nitpick comments (2)
.github/workflows/docker-image-alpha.yml (2)
38-38: Avoid hard-coding the Docker Hub namespace; parameterize the image name.This makes the workflow portable across forks/environments and removes a hard dependency on a specific account.
- images: calciumion/new-api + images: docker.io/${{ secrets.DOCKERHUB_USERNAME }}/new-apiIf you prefer non-secret config, consider a repo variable instead:
# Settings → Variables → Actions # vars.DOCKERHUB_NAMESPACE="calciumion" with: images: docker.io/${{ vars.DOCKERHUB_NAMESPACE }}/new-api
43-50: Optional: add SBOM and provenance attestations for supply-chain integrity.If you want traceability, enable these in the build step.
- name: Build and push Docker image uses: docker/build-push-action@v5 with: context: . platforms: linux/amd64,linux/arm64 push: true tags: ${{ steps.meta.outputs.tags }} labels: ${{ steps.meta.outputs.labels }} + sbom: true + provenance: true
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docker-image-alpha.yml(2 hunks)
🔇 Additional comments (2)
.github/workflows/docker-image-alpha.yml (2)
14-15: Scope reduction to Docker Hub-only looks good.Job rename and focus on Docker Hub are consistent and straightforward.
34-41: Metadata step is fine for alpha tagging.Tags and labels from docker/metadata-action are configured appropriately for alpha and date+sha tags.
| push_to_dockerhub: | ||
| name: Push Docker image to Docker Hub | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Pin least-privilege GITHUB_TOKEN permissions at the job level.
Now that GHCR push is removed, explicitly constrain token permissions to avoid unexpected future defaults.
jobs:
push_to_dockerhub:
name: Push Docker image to Docker Hub
+ permissions:
+ contents: read
runs-on: ubuntu-latest📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| push_to_dockerhub: | |
| name: Push Docker image to Docker Hub | |
| runs-on: ubuntu-latest | |
| push_to_dockerhub: | |
| name: Push Docker image to Docker Hub | |
| permissions: | |
| contents: read | |
| runs-on: ubuntu-latest |
🤖 Prompt for AI Agents
In .github/workflows/docker-image-alpha.yml around lines 14 to 16, the job
push_to_dockerhub lacks explicit GITHUB_TOKEN permission settings. To enforce
least-privilege access, add a permissions block at the job level specifying only
the necessary permissions for this job, such as write access to packages if
pushing images, and remove any broader default permissions. This prevents
unexpected permission escalations in the future.
feats:replace GLM-4v authentication headers to support customize api key
2 participants
According to the official documentation (https://docs.bigmodel.cn/cn/guide/develop/http/introduction), JWT generation is not required, so it has been replaced with a more compatible approach.
Summary by CodeRabbit
Refactor
Chores
Chores