🐛 Reject non-UTF-8 skill resources

Author: frostime

kernel/mcp/server_test.go

@@ -22,11 +22,14 @@ import (
 	"io"
 	"net/http"
 	"net/http/httptest"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
 	mcpsdk "github.com/modelcontextprotocol/go-sdk/mcp"
 	"github.com/siyuan-note/siyuan/kernel/mcp/tools"
+	"github.com/siyuan-note/siyuan/kernel/util"
 )
 
 func newTestHTTPServer(t *testing.T) (*mcpsdk.Server, *httptest.Server) {
@@ -392,6 +395,44 @@ func TestToolInputAndOutputValidation(t *testing.T) {
 	}
 }
 
+func TestSkillResourceContentSurvivesMCPConversion(t *testing.T) {
+	originalDataDir, originalHomeDir := util.DataDir, util.HomeDir
+	root := t.TempDir()
+	util.DataDir = filepath.Join(root, "workspace", "data")
+	util.HomeDir = filepath.Join(root, "home")
+	t.Cleanup(func() {
+		util.DataDir, util.HomeDir = originalDataDir, originalHomeDir
+	})
+
+	skillDir := filepath.Join(util.SkillsDir(), "transport-skill")
+	if err := os.MkdirAll(filepath.Join(skillDir, "references"), 0755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"),
+		[]byte("---\nname: transport-skill\ndescription: transport test\n---\nbody"), 0644); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(skillDir, "references", "valid.txt"), []byte("中文内容"), 0644); err != nil {
+		t.Fatal(err)
+	}
+
+	result, err := tools.SkillTool.Handler(map[string]any{
+		"action": "load",
+		"name":   "transport-skill/references/valid.txt",
+	})
+	if err != nil || result.IsError || len(result.Content) != 1 {
+		t.Fatalf("unexpected Skill resource result: %#v, %v", result, err)
+	}
+	converted, err := convertContentItem(result.Content[0])
+	if err != nil {
+		t.Fatal(err)
+	}
+	text, ok := converted.(*mcpsdk.TextContent)
+	if !ok || text.Text != "<skill_resource skill=\"transport-skill\" path=\"references/valid.txt\">\n\n中文内容\n\n</skill_resource>" {
+		t.Fatalf("Skill resource changed during MCP conversion: %#v", converted)
+	}
+}
+
 func TestNonTextToolContentIsPreserved(t *testing.T) {
 	server, httpServer := newTestHTTPServer(t)
 	var imageContent tools.ContentItem

kernel/mcp/tools/skill.go

@@ -101,12 +101,11 @@ func skillLoad(args map[string]any) (CallToolResult, error) {
 
 	var result string
 	if loaded.ResourcePath == "" {
-		// 变量（非敏感）在技能正文注入对话时解析。
+		// Variables.Resolve 还会匹配 $NAME 和 ${NAME}。
 		// 资源可能包含脚本或模板，因此只解析技能正文，避免误改资源内容。
 		loaded.Content = model.Conf.Variables.Resolve(loaded.Content)
 		result = formatSkillContent(loaded)
 	} else {
-		// 不做变量解析注入，避免误杀 $NAME ${NAME} 等 reference 文件中常见模式
 		result = formatSkillResource(loaded)
 	}
 	return CallToolResult{

kernel/util/skill.go

@@ -28,6 +28,7 @@ import (
 	"regexp"
 	"sort"
 	"strings"
+	"unicode/utf8"
 
 	"github.com/88250/gulu"
 	"github.com/siyuan-note/filelock"
@@ -386,6 +387,9 @@ func readSkillResource(skillDir, skillName, resource string) (string, error) {
 	if len(data) > maxSkillResourceBytes {
 		return "", fmt.Errorf("skill resource exceeds the %d byte limit: %s/%s", maxSkillResourceBytes, skillName, resource)
 	}
+	if !utf8.Valid(data) {
+		return "", fmt.Errorf("skill resource is not valid UTF-8: %s/%s", skillName, resource)
+	}
 	return string(data), nil
 }
 

kernel/util/skill_test.go

@@ -222,11 +222,10 @@ func TestLoadSkillReadsOnlyEnabledUserResources(t *testing.T) {
 	}
 }
 
-func TestLoadSkillRejectsUnsafeOrOversizedResources(t *testing.T) {
+func TestLoadSkillRejectsUnsafeOrUnsupportedResources(t *testing.T) {
 	workspaceRoot, _ := setSkillTestRoots(t)
 	writeSkill(t, workspaceRoot, "safe", "safe", "description", "body")
-	legacyEncoded := []byte{0xd6, 0xd0, 0xce, 0xc4}
-	writeSkillResource(t, workspaceRoot, "safe", "legacy-gbk.txt", legacyEncoded)
+	writeSkillResource(t, workspaceRoot, "safe", "invalid-utf8.txt", []byte{0xd6, 0xd0, 0xce, 0xc4})
 	writeSkillResource(t, workspaceRoot, "safe", "limit.txt", bytes.Repeat([]byte("x"), maxSkillResourceBytes))
 	writeSkillResource(t, workspaceRoot, "safe", "large.txt", bytes.Repeat([]byte("x"), maxSkillResourceBytes+1))
 
@@ -240,9 +239,9 @@ func TestLoadSkillRejectsUnsafeOrOversizedResources(t *testing.T) {
 			t.Errorf("LoadSkill(%q) error = %v", locator, err)
 		}
 	}
-	legacy, err := LoadSkill("safe/legacy-gbk.txt", nil)
-	if err != nil || !bytes.Equal([]byte(legacy.Content), legacyEncoded) {
-		t.Fatalf("legacy-encoded resource = %v, %v", []byte(legacy.Content), err)
+	if _, err := LoadSkill("safe/invalid-utf8.txt", nil); err == nil ||
+		!strings.Contains(err.Error(), "skill resource is not valid UTF-8") {
+		t.Fatalf("invalid UTF-8 resource error = %v", err)
 	}
 	atLimit, err := LoadSkill("safe/limit.txt", nil)
 	if err != nil || len(atLimit.Content) != maxSkillResourceBytes {