From e023c165b674f4286c1c1bd50ccdb36f07fde2f5 Mon Sep 17 00:00:00 2001 From: nickmopen Date: Fri, 29 May 2026 05:08:55 +0300 Subject: [PATCH] Fix(kb): enforce tenant authorization on UpdateMetadataSetting (#15268) (#15270) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Closes #15268. The `UpdateMetadataSetting` handler at `internal/handler/kb.go:126` retrieved the authenticated user via `GetUser(c)` but discarded the user object (`_, errorCode, errorMessage := GetUser(c)`), then forwarded the caller-supplied `kb_id` straight to the service layer with no ownership check. Any authenticated user could mutate the `parser_config` / metadata of any knowledge base in the system by guessing or harvesting a `kb_id` — a classic IDOR (CWE-284, OWASP A01). This is the only handler in `internal/handler/kb.go` missing the check; every sibling (`ListTags`, `ListTagsFromKbs`, `RenameTag`, `KnowledgeGraph`, `DeleteKnowledgeGraph`, `GetMeta`, `GetBasicInfo`) already calls `h.kbService.Accessible(kbID, user.ID)`. The same defensive check on the document preview endpoint was added in PR #14625 — this PR closes the matching gap on the KB metadata endpoint. --------- Co-authored-by: Jin Hai --- internal/handler/kb.go | 13 +++++++++++-- internal/service/kb.go | 11 +++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/handler/kb.go b/internal/handler/kb.go index c44fa741c2..5ba5e0543a 100644 --- a/internal/handler/kb.go +++ b/internal/handler/kb.go @@ -124,7 +124,7 @@ func (h *KnowledgebaseHandler) UpdateKB(c *gin.Context) { // @Success 200 {object} map[string]interface{} // @Router /v1/kb/update_metadata_setting [post] func (h *KnowledgebaseHandler) UpdateMetadataSetting(c *gin.Context) { - _, errorCode, errorMessage := GetUser(c) + user, errorCode, errorMessage := GetUser(c) if errorCode != common.CodeSuccess { jsonError(c, errorCode, errorMessage) return @@ -136,8 +136,17 @@ func (h *KnowledgebaseHandler) UpdateMetadataSetting(c *gin.Context) { return } - result, code, err := h.kbService.UpdateMetadataSetting(&req) + if !h.kbService.Accessible(req.KBID, user.ID) { + jsonError(c, common.CodeAuthenticationError, "No authorization.") + return + } + + result, code, err := h.kbService.UpdateMetadataSetting(&req, user.ID) if err != nil { + if strings.Contains(err.Error(), "authorized") { + jsonError(c, common.CodeAuthenticationError, err.Error()) + return + } jsonError(c, code, err.Error()) return } diff --git a/internal/service/kb.go b/internal/service/kb.go index 8913bf0a22..e97922b2a3 100644 --- a/internal/service/kb.go +++ b/internal/service/kb.go @@ -164,8 +164,15 @@ func (s *KnowledgebaseService) UpdateKB(req *UpdateKBRequest, userID string) (ma return result, common.CodeSuccess, nil } -// UpdateMetadataSetting updates the metadata settings for a knowledge base -func (s *KnowledgebaseService) UpdateMetadataSetting(req *UpdateMetadataSettingRequest) (map[string]interface{}, common.ErrorCode, error) { +// UpdateMetadataSetting updates the metadata settings for a knowledge base. +// The userID must be a member of the owning tenant; this is the same authorization +// boundary applied by GetDetail and the handler-level guard, duplicated here so +// the security check cannot be regressed by future handler refactors that drop it. +func (s *KnowledgebaseService) UpdateMetadataSetting(req *UpdateMetadataSettingRequest, userID string) (map[string]interface{}, common.ErrorCode, error) { + if !s.kbDAO.Accessible(req.KBID, userID) { + return nil, common.CodeOperatingError, errors.New("only owner of dataset authorized for this operation") + } + kb, err := s.kbDAO.GetByID(req.KBID) if err != nil { return nil, common.CodeDataError, errors.New("database error (knowledgebase not found)")