mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-13 20:14:50 +00:00
fix(vision): fix Lark inline image parsing and MCP bridge read_image (#396)
* fix(vision): fix Lark inline image parsing and MCP bridge read_image
Two fixes for image reading through Lark channel with claude-cli provider:
1. Lark post messages use flat format {"content":[[...]]} but resolvePostElements
only handled language-wrapped format {"zh_cn":{"content":[...]}}. Now checks
flat format first, and skips non-map values in fallback iteration.
2. read_image tool failed via MCP bridge because bridge creates fresh HTTP context
losing WithMediaImages. Fix: enrich <media:image> tags with path attribute
(like documents), pass workspace through X-Workspace bridge header with HMAC,
so read_image can load images from disk via path parameter.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix(security): add HMAC backward compat and guard workspace injection
- VerifyBridgeContext falls back to old signature format (without workspace)
for sessions with pre-existing MCP configs, preventing 403 on server restart
- Workspace context injection now requires agent context (agentID/userID)
to prevent unauthenticated path injection bypassing HMAC
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: viettranx <viettranx@gmail.com>
This commit is contained in:
7 files changed
+70
-31
No files matched your search
@@ -724,6 +724,7 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error)
|
||||
providers.OptChannel: req.Channel,
|
||||
providers.OptChatID: req.ChatID,
|
||||
providers.OptPeerKind: req.PeerKind,
|
||||
providers.OptWorkspace: tools.ToolWorkspaceFromCtx(ctx),
|
||||
},
|
||||
}
|
||||
if l.thinkingLevel != "" && l.thinkingLevel != "off" {
|
||||
|
||||
+10
-4
@@ -291,8 +291,10 @@ func (l *Loop) enrichVideoIDs(messages []providers.Message, refs []providers.Med
|
||||
}
|
||||
|
||||
// enrichImageIDs updates the last user message to embed persisted media IDs
|
||||
// in <media:image> tags so the LLM knows images were received and stored.
|
||||
// Without this, the LLM sees plain <media:image> and cannot confirm image availability.
|
||||
// and file paths in <media:image> tags so the LLM knows images were received
|
||||
// and stored. The path attribute allows tools called via MCP bridge (e.g.
|
||||
// claude-cli) to access images via read_image(path=...) even though the
|
||||
// bridge context does not carry WithMediaImages.
|
||||
// Iterates refs in reverse order so that when multiple images are present,
|
||||
// each ref maps to the correct positional tag (last ref → last tag, etc.).
|
||||
func (l *Loop) enrichImageIDs(messages []providers.Message, refs []providers.MediaRef) {
|
||||
@@ -318,11 +320,15 @@ func (l *Loop) enrichImageIDs(messages []providers.Message, refs []providers.Med
|
||||
continue
|
||||
}
|
||||
idAttr := fmt.Sprintf(" id=%q", ref.ID)
|
||||
pathAttr := ""
|
||||
if ref.Path != "" {
|
||||
pathAttr = fmt.Sprintf(" path=%q", ref.Path)
|
||||
}
|
||||
|
||||
// Replace the LAST bare <media:image> with <media:image id="uuid">
|
||||
// Replace the LAST bare <media:image> with <media:image id="uuid" path="...">
|
||||
bare := "<media:image>"
|
||||
if idx := strings.LastIndex(content, bare); idx >= 0 {
|
||||
content = content[:idx] + "<media:image" + idAttr + ">" + content[idx+len(bare):]
|
||||
content = content[:idx] + "<media:image" + idAttr + pathAttr + ">" + content[idx+len(bare):]
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
@@ -100,13 +100,21 @@ func parseMessageContent(rawContent, messageType string) string {
|
||||
}
|
||||
|
||||
// resolvePostElements extracts the flattened element list from a Lark post message JSON.
|
||||
// Handles language variant lookup (zh_cn → en_us → any first key).
|
||||
// Handles two formats:
|
||||
// 1. Language-wrapped: {"zh_cn": {"title":"...", "content": [[...]]}}
|
||||
// 2. Flat (no language key): {"title":"...", "content": [[...]]}
|
||||
func resolvePostElements(rawContent string) []any {
|
||||
var post map[string]any
|
||||
if err := json.Unmarshal([]byte(rawContent), &post); err != nil {
|
||||
return nil
|
||||
}
|
||||
|
||||
// Format 2: flat — "content" at top level (no language wrapper).
|
||||
if contentArr, ok := post["content"].([]any); ok {
|
||||
return contentArr
|
||||
}
|
||||
|
||||
// Format 1: language-wrapped — try known locales, then first map value.
|
||||
var langContent any
|
||||
for _, lang := range []string{"zh_cn", "en_us"} {
|
||||
if lc, ok := post[lang]; ok {
|
||||
@@ -116,8 +124,10 @@ func resolvePostElements(rawContent string) []any {
|
||||
}
|
||||
if langContent == nil {
|
||||
for _, v := range post {
|
||||
langContent = v
|
||||
break
|
||||
if _, ok := v.(map[string]any); ok {
|
||||
langContent = v
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
if langContent == nil {
|
||||
|
||||
@@ -329,10 +329,11 @@ func (s *Server) BuildMux() *http.ServeMux {
|
||||
return mux
|
||||
}
|
||||
|
||||
// bridgeContextMiddleware extracts X-Agent-ID and X-User-ID headers from the
|
||||
// MCP bridge request and injects them into the context so bridge tools can
|
||||
// access agent/user scope. When a gateway token is configured, the context
|
||||
// headers must be accompanied by a valid X-Bridge-Sig HMAC to prevent forgery.
|
||||
// bridgeContextMiddleware extracts X-Agent-ID, X-User-ID, and X-Workspace headers
|
||||
// from the MCP bridge request and injects them into the context so bridge tools can
|
||||
// access agent/user scope and resolve workspace-relative paths.
|
||||
// When a gateway token is configured, the context headers must be accompanied by
|
||||
// a valid X-Bridge-Sig HMAC to prevent forgery.
|
||||
func bridgeContextMiddleware(gatewayToken string, next http.Handler) http.Handler {
|
||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||
ctx := r.Context()
|
||||
@@ -341,6 +342,7 @@ func bridgeContextMiddleware(gatewayToken string, next http.Handler) http.Handle
|
||||
channel := r.Header.Get("X-Channel")
|
||||
chatID := r.Header.Get("X-Chat-ID")
|
||||
peerKind := r.Header.Get("X-Peer-Kind")
|
||||
workspace := r.Header.Get("X-Workspace")
|
||||
|
||||
if agentIDStr != "" || userID != "" {
|
||||
// Reject context headers when no gateway token — prevents unauthenticated impersonation.
|
||||
@@ -353,7 +355,7 @@ func bridgeContextMiddleware(gatewayToken string, next http.Handler) http.Handle
|
||||
|
||||
// Verify HMAC signature over all context fields.
|
||||
sig := r.Header.Get("X-Bridge-Sig")
|
||||
if !providers.VerifyBridgeContext(gatewayToken, agentIDStr, userID, channel, chatID, peerKind, sig) {
|
||||
if !providers.VerifyBridgeContext(gatewayToken, agentIDStr, userID, channel, chatID, peerKind, workspace, sig) {
|
||||
slog.Warn("security.mcp_bridge: invalid bridge context signature",
|
||||
"agent_id", agentIDStr, "user_id", userID)
|
||||
http.Error(w, `{"error":"invalid bridge context signature"}`, http.StatusForbidden)
|
||||
@@ -380,6 +382,11 @@ func bridgeContextMiddleware(gatewayToken string, next http.Handler) http.Handle
|
||||
if peerKind != "" {
|
||||
ctx = tools.WithToolPeerKind(ctx, peerKind)
|
||||
}
|
||||
// Inject workspace so bridge tools (read_image, read_file, etc.) can resolve paths.
|
||||
// Only when agent context is present (HMAC-protected) to prevent unauthenticated path injection.
|
||||
if workspace != "" && (agentIDStr != "" || userID != "") {
|
||||
ctx = tools.WithToolWorkspace(ctx, workspace)
|
||||
}
|
||||
|
||||
next.ServeHTTP(w, r.WithContext(ctx))
|
||||
})
|
||||
|
||||
@@ -28,6 +28,9 @@ const OptChatID = "chat_id"
|
||||
// OptPeerKind passes the peer kind (direct/group) for MCP bridge context.
|
||||
const OptPeerKind = "peer_kind"
|
||||
|
||||
// OptWorkspace passes the agent workspace path so MCP bridge tools can resolve file paths.
|
||||
const OptWorkspace = "workspace"
|
||||
|
||||
// ClaudeCLIProvider implements Provider by shelling out to the `claude` CLI binary.
|
||||
// It acts as a thin proxy: CLI manages session history, tool execution, and context.
|
||||
// GoClaw only forwards the latest user message and streams back the response.
|
||||
|
||||
@@ -84,11 +84,12 @@ func mcpConfigBaseDir() string {
|
||||
|
||||
// BridgeContext holds per-call context for MCP bridge headers.
|
||||
type BridgeContext struct {
|
||||
AgentID string
|
||||
UserID string
|
||||
Channel string
|
||||
ChatID string
|
||||
PeerKind string
|
||||
AgentID string
|
||||
UserID string
|
||||
Channel string
|
||||
ChatID string
|
||||
PeerKind string
|
||||
Workspace string
|
||||
}
|
||||
|
||||
// WriteMCPConfig writes a per-session MCP config file with agent context headers.
|
||||
@@ -96,10 +97,10 @@ type BridgeContext struct {
|
||||
// outside the agent's workDir so tokens are not exposed.
|
||||
// Skips write if content is unchanged. Returns the file path.
|
||||
func (d *MCPConfigData) WriteMCPConfig(ctx context.Context, sessionKey string, bc BridgeContext) string {
|
||||
return d.writeMCPConfigInternal(ctx, sessionKey, bc.AgentID, bc.UserID, bc.Channel, bc.ChatID, bc.PeerKind)
|
||||
return d.writeMCPConfigInternal(ctx, sessionKey, bc.AgentID, bc.UserID, bc.Channel, bc.ChatID, bc.PeerKind, bc.Workspace)
|
||||
}
|
||||
|
||||
func (d *MCPConfigData) writeMCPConfigInternal(ctx context.Context, sessionKey, agentID, userID, channel, chatID, peerKind string) string {
|
||||
func (d *MCPConfigData) writeMCPConfigInternal(ctx context.Context, sessionKey, agentID, userID, channel, chatID, peerKind, workspace string) string {
|
||||
if d == nil || (len(d.Servers) == 0 && d.GatewayAddr == "" && d.AgentMCPLookup == nil) {
|
||||
return ""
|
||||
}
|
||||
@@ -143,9 +144,12 @@ func (d *MCPConfigData) writeMCPConfigInternal(ctx context.Context, sessionKey,
|
||||
if peerKind != "" && !strings.ContainsAny(peerKind, "\r\n\x00") {
|
||||
headers["X-Peer-Kind"] = peerKind
|
||||
}
|
||||
if workspace != "" && !strings.ContainsAny(workspace, "\r\n\x00") {
|
||||
headers["X-Workspace"] = workspace
|
||||
}
|
||||
// HMAC signature over all context fields to prevent header forgery
|
||||
if d.GatewayToken != "" && (agentID != "" || userID != "") {
|
||||
headers["X-Bridge-Sig"] = SignBridgeContext(d.GatewayToken, agentID, userID, channel, chatID, peerKind)
|
||||
headers["X-Bridge-Sig"] = SignBridgeContext(d.GatewayToken, agentID, userID, channel, chatID, peerKind, workspace)
|
||||
}
|
||||
|
||||
bridgeEntry := map[string]any{
|
||||
@@ -248,15 +252,22 @@ func sanitizePathSegment(s string) string {
|
||||
}
|
||||
|
||||
// SignBridgeContext computes HMAC-SHA256 over all bridge context fields to prevent forgery.
|
||||
// Payload: agentID|userID|channel|chatID|peerKind
|
||||
func SignBridgeContext(key, agentID, userID, channel, chatID, peerKind string) string {
|
||||
// Payload: agentID|userID|channel|chatID|peerKind|workspace
|
||||
func SignBridgeContext(key, agentID, userID, channel, chatID, peerKind, workspace string) string {
|
||||
mac := hmac.New(sha256.New, []byte(key))
|
||||
mac.Write([]byte(agentID + "|" + userID + "|" + channel + "|" + chatID + "|" + peerKind))
|
||||
mac.Write([]byte(agentID + "|" + userID + "|" + channel + "|" + chatID + "|" + peerKind + "|" + workspace))
|
||||
return hex.EncodeToString(mac.Sum(nil))
|
||||
}
|
||||
|
||||
// VerifyBridgeContext checks the HMAC signature against the expected bridge context.
|
||||
func VerifyBridgeContext(key, agentID, userID, channel, chatID, peerKind, sig string) bool {
|
||||
expected := SignBridgeContext(key, agentID, userID, channel, chatID, peerKind)
|
||||
return hmac.Equal([]byte(expected), []byte(sig))
|
||||
// Falls back to old format (without workspace) for backward compatibility with
|
||||
// sessions whose MCP config was written before the workspace field was added.
|
||||
func VerifyBridgeContext(key, agentID, userID, channel, chatID, peerKind, workspace, sig string) bool {
|
||||
expected := SignBridgeContext(key, agentID, userID, channel, chatID, peerKind, workspace)
|
||||
if hmac.Equal([]byte(expected), []byte(sig)) {
|
||||
return true
|
||||
}
|
||||
// Fallback: verify old format (without workspace) for backward compat.
|
||||
old := SignBridgeContext(key, agentID, userID, channel, chatID, peerKind, "")
|
||||
return hmac.Equal([]byte(old), []byte(sig))
|
||||
}
|
||||
@@ -160,11 +160,12 @@ func extractBoolOpt(opts map[string]any, key string) bool {
|
||||
// bridgeContextFromOpts builds a BridgeContext from the Options map.
|
||||
func bridgeContextFromOpts(opts map[string]any) BridgeContext {
|
||||
return BridgeContext{
|
||||
AgentID: extractStringOpt(opts, OptAgentID),
|
||||
UserID: extractStringOpt(opts, OptUserID),
|
||||
Channel: extractStringOpt(opts, OptChannel),
|
||||
ChatID: extractStringOpt(opts, OptChatID),
|
||||
PeerKind: extractStringOpt(opts, OptPeerKind),
|
||||
AgentID: extractStringOpt(opts, OptAgentID),
|
||||
UserID: extractStringOpt(opts, OptUserID),
|
||||
Channel: extractStringOpt(opts, OptChannel),
|
||||
ChatID: extractStringOpt(opts, OptChatID),
|
||||
PeerKind: extractStringOpt(opts, OptPeerKind),
|
||||
Workspace: extractStringOpt(opts, OptWorkspace),
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user