From c59b8ec7f7025115aea782d64cf4bb87d85697e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Yingyi=20/=20=E9=A2=96=E9=80=B8?= <49649786+Zuoqiu-Yingyi@users.noreply.github.com> Date: Sat, 9 May 2026 19:38:54 +0800 Subject: [PATCH] :art: Harden RPC handling and improve error reporting in kernel plugin (#17655) * perf(kernel-plugin): strengthen RPC, sandbox, and form parsing Validate and harden plugin RPC and request handling: ensure RPC API call first argument is a string; treat missing method using HasValue(); return InvalidParams for malformed params; bail out early when kernel is incompatible or missing. Fix sandbox promise invocation to return after reporting errors to avoid continuing on nil/invalid values. Change RequestForm files to []*RequestFile, allocate pointer entries, properly open/read/close uploaded files, and clone request headers before modifying them. These changes prevent nil derefs, resource leaks, and improve error reporting. * perf(kernel-plugin): Skip empty Content-Type; use safe type assertions Avoid setting an empty Content-Type header in the proxy when gin.Context.ContentType() is empty. Replace unsafe type assertions with comma-ok checks when converting request and file body bytes to Data objects to prevent panics on unexpected types or nil pointers. Also comment out assignments of c.Request.Context().Err() in plugin request handlers to avoid overwriting other error state on context cancellation. Affected files: kernel/api/network.go, kernel/plugin/plugin.go, kernel/plugin/sandbox.go. --- kernel/api/network.go | 4 +++- kernel/plugin/api_rpc.go | 3 +++ kernel/plugin/manager.go | 5 +++++ kernel/plugin/plugin.go | 4 ++-- kernel/plugin/rpc.go | 6 +++--- kernel/plugin/sandbox.go | 10 ++++++---- kernel/plugin/server.go | 39 +++++++++++++++++++++------------------ 7 files changed, 43 insertions(+), 28 deletions(-) diff --git a/kernel/api/network.go b/kernel/api/network.go index 70561c6fb..8af8ba625 100644 --- a/kernel/api/network.go +++ b/kernel/api/network.go @@ -447,7 +447,9 @@ func httpProxy(c *gin.Context) { } proxyReq.ContentLength = c.Request.ContentLength - proxyReq.Header.Set("Content-Type", c.ContentType()) + if c.ContentType() != "" { + proxyReq.Header.Set("Content-Type", c.ContentType()) + } for k, vs := range *targetHeaders { for _, v := range vs { diff --git a/kernel/plugin/api_rpc.go b/kernel/plugin/api_rpc.go index 8d0d34b42..5923d80dc 100644 --- a/kernel/plugin/api_rpc.go +++ b/kernel/plugin/api_rpc.go @@ -139,6 +139,9 @@ func injectRpc(p *KernelPlugin, rt *goja.Runtime, siyuan *goja.Object) (err erro var method string if m := call.Argument(0); goja.IsString(m) { method = m.String() + } else { + err = fmt.Errorf("first argument must be method name string") + return } var params util.Optional[any] diff --git a/kernel/plugin/manager.go b/kernel/plugin/manager.go index 8d59a0ecc..7e657afd4 100644 --- a/kernel/plugin/manager.go +++ b/kernel/plugin/manager.go @@ -142,6 +142,11 @@ func (m *PluginManager) StartPlugin(petal *model.Petal) (ok bool) { } }() + if petal.Kernel.Incompatible || !petal.Kernel.Existed { + ok = false + return + } + pluginMu := m.getPluginMu(petal.Name) pluginMu.Lock() defer pluginMu.Unlock() diff --git a/kernel/plugin/plugin.go b/kernel/plugin/plugin.go index 432f7d346..e002b9dc6 100644 --- a/kernel/plugin/plugin.go +++ b/kernel/plugin/plugin.go @@ -760,7 +760,7 @@ func (p *KernelPlugin) handleHttpRequest(c *gin.Context, request *Request, scope response = result.Value } case <-c.Request.Context().Done(): - err = c.Request.Context().Err() + // err = c.Request.Context().Err() } return } @@ -1221,7 +1221,7 @@ func (p *KernelPlugin) handleServerSentEventRequest(c *gin.Context, request *Req case <-ctx.Done(): return case <-c.Request.Context().Done(): - err = c.Request.Context().Err() + // err = c.Request.Context().Err() return case handlerErr := <-done: if handlerErr != nil { diff --git a/kernel/plugin/rpc.go b/kernel/plugin/rpc.go index 64c0ef257..11ffc4f2c 100644 --- a/kernel/plugin/rpc.go +++ b/kernel/plugin/rpc.go @@ -115,7 +115,7 @@ func (r *JsonRpcRequest) UnmarshalJSON(data []byte) error { } // Validate method field - if !request.Method.Exists { + if !request.Method.HasValue() { return fmt.Errorf("missing method field") } @@ -148,8 +148,8 @@ func (r *JsonRpcRequest) Validate() *JsonRpcError { } else if _, ok := r.Params.Value.(map[string]any); ok { } else { return &JsonRpcError{ - Code: JsonRpcErrorCodeInvalidRequest, - Message: JsonRpcErrorInvalidRequest.Message, + Code: JsonRpcErrorCodeInvalidParams, + Message: JsonRpcErrorInvalidParams.Message, Data: "Invalid params: must be array or object if present", } } diff --git a/kernel/plugin/sandbox.go b/kernel/plugin/sandbox.go index a975c2ced..f414de802 100644 --- a/kernel/plugin/sandbox.go +++ b/kernel/plugin/sandbox.go @@ -366,12 +366,14 @@ func invokeFunction(callback func(rt *goja.Runtime, result *CallResult), rt *goj resultObj := resultJs.ToObject(rt) if resultObj == nil { callback(rt, &CallResult{Error: fmt.Errorf("expected promise object, got %T", result)}) + return } thenValue := resultObj.Get("then") then, ok := goja.AssertFunction(thenValue) if !ok { callback(rt, &CallResult{Error: fmt.Errorf("'promise.then property is not a function")}) + return } then(resultObj, rt.ToValue(func(call goja.FunctionCall, rt *goja.Runtime) { @@ -503,8 +505,8 @@ func getRequestHandler(rt *goja.Runtime, scope AccessScope, requestType RequestT // requestGoToJs converts a Go Request to a JavaScript value. func requestGoToJs(p *KernelPlugin, rt *goja.Runtime, request *Request) (jsRequest goja.Value, err error) { // convert body raw data to js object - if request.Request.Body.Data != nil { - request.Request.Body.Data, err = NewDataObject(p, rt, *request.Request.Body.Data.(*[]byte)) + if data, ok := request.Request.Body.Data.(*[]byte); ok && data != nil { + request.Request.Body.Data, err = NewDataObject(p, rt, *data) if err != nil { return } @@ -514,8 +516,8 @@ func requestGoToJs(p *KernelPlugin, rt *goja.Runtime, request *Request) (jsReque if request.Request.Body.Form != nil { for _, fileList := range request.Request.Body.Form.File { for _, file := range fileList { - if file.Data != nil { - file.Data, err = NewDataObject(p, rt, *file.Data.(*[]byte)) + if data, ok := file.Data.(*[]byte); ok && data != nil { + file.Data, err = NewDataObject(p, rt, *data) if err != nil { return } diff --git a/kernel/plugin/server.go b/kernel/plugin/server.go index 9984945b7..56190f7d0 100644 --- a/kernel/plugin/server.go +++ b/kernel/plugin/server.go @@ -106,8 +106,8 @@ type RequestBody struct { } type RequestForm struct { - Value map[string][]string `json:"values"` // e.g. {"field1": ["value1"], "field2": ["value2-1", "value2-2"]} - File map[string][]RequestFile `json:"files"` // e.g. {"file1": [{"Filename": "hello.txt", "Headers": {"Content-Disposition": ["form-data; name=\"file1\"; filename=\"hello.txt\""], "Content-Type": ["text/plain"]}, "Size": 123, "Data": []byte{...}}]} + Value map[string][]string `json:"values"` // e.g. {"field1": ["value1"], "field2": ["value2-1", "value2-2"]} + File map[string][]*RequestFile `json:"files"` // e.g. {"file1": [{"Filename": "hello.txt", "Headers": {"Content-Disposition": ["form-data; name=\"file1\"; filename=\"hello.txt\""], "Content-Type": ["text/plain"]}, "Size": 123, "Data": []byte{...}}]} } type RequestFile struct { @@ -185,29 +185,32 @@ func parseRequest(c *gin.Context) (request *Request, err error) { // multipart/form-data form = &RequestForm{ Value: multipartForm.Value, - File: make(map[string][]RequestFile), + File: make(map[string][]*RequestFile), } for partName, fileHandlers := range multipartForm.File { - files := make([]RequestFile, len(fileHandlers)) + files := make([]*RequestFile, len(fileHandlers)) form.File[partName] = files for i, handler := range fileHandlers { - files[i].Filename = handler.Filename - files[i].Headers = handler.Header - files[i].Size = handler.Size - if file, openErr := handler.Open(); openErr != nil { + files[i] = &RequestFile{ + Filename: handler.Filename, + Headers: handler.Header, + Size: handler.Size, + } + file, openErr := handler.Open() + if openErr != nil { err = fmt.Errorf("open form part [%s] file [%s] error: %s", partName, handler.Filename, openErr.Error()) return - } else { - content := make([]byte, handler.Size) - if n, readErr := file.Read(content); readErr != nil { - err = fmt.Errorf("read form part [%s] file [%s] error: %s", partName, handler.Filename, readErr.Error()) - return - } else { - fileData := content[:n] - files[i].Data = &fileData - } } + content := make([]byte, handler.Size) + n, readErr := file.Read(content) + file.Close() + if readErr != nil { + err = fmt.Errorf("read form part [%s] file [%s] error: %s", partName, handler.Filename, readErr.Error()) + return + } + fileData := content[:n] + files[i].Data = &fileData } } } else if len(c.Request.PostForm) > 0 { @@ -236,7 +239,7 @@ func parseRequest(c *gin.Context) (request *Request, err error) { } } - headers := map[string][]string(c.Request.Header) + headers := map[string][]string(c.Request.Header.Clone()) delete(headers, "Cookie") delete(headers, "Authorization")