tiennm99/goclaw
1
0 Fork 0
mirror of https://github.com/tiennm99/goclaw.git synced 2026-10-19 14:15:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(security): add tenant membership check to tenant-config endpoints

Browse Source 
Prevent cross-tenant config manipulation by verifying caller has
owner/admin role within the target tenant before allowing set/delete
operations. Also adds MaxBytesReader, audit trail, and fixes middleware
consistency between skills and builtin_tools endpoints.
This commit is contained in:
viettranx committed 2026-03-25 10:21:39 +07:00
1 parent c5164255ef
commit a9aedc843e
4 files changed
+78 -26
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

cmd/gateway_http_handlers.go
+2 -2
View File
@@ -33,7 +33,7 @@ func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir
if pgSkills, ok := stores.Skills.(*pg.PGSkillStore); ok {
dirs := pgSkills.Dirs()
if len(dirs) > 0 {
skillsH = httpapi.NewSkillsHandler(pgSkills, dirs[0], dataDir, bundledSkillsDir, msgBus, stores.SkillTenantCfgs)
skillsH = httpapi.NewSkillsHandler(pgSkills, dirs[0], dataDir, bundledSkillsDir, msgBus, stores.SkillTenantCfgs, stores.Tenants)
}
}
}
@@ -72,7 +72,7 @@ func wireHTTP(stores *store.Stores, defaultWorkspace, dataDir, bundledSkillsDir
}
if stores != nil && stores.BuiltinTools != nil {
builtinToolsH = httpapi.NewBuiltinToolsHandler(stores.BuiltinTools, stores.BuiltinToolTenantCfgs, msgBus)
builtinToolsH = httpapi.NewBuiltinToolsHandler(stores.BuiltinTools, stores.BuiltinToolTenantCfgs, stores.Tenants, msgBus)
}
if stores != nil && stores.PendingMessages != nil {
internal/http/builtin_tools.go
+12 -11
View File
@@ -18,12 +18,13 @@ import (
type BuiltinToolsHandler struct {
store store.BuiltinToolStore
tenantCfgStore store.BuiltinToolTenantConfigStore
tenantStore store.TenantStore
msgBus *bus.MessageBus
}
// NewBuiltinToolsHandler creates a handler for built-in tool management endpoints.
func NewBuiltinToolsHandler(s store.BuiltinToolStore, tenantCfgs store.BuiltinToolTenantConfigStore, msgBus *bus.MessageBus) *BuiltinToolsHandler {
return &BuiltinToolsHandler{store: s, tenantCfgStore: tenantCfgs, msgBus: msgBus}
func NewBuiltinToolsHandler(s store.BuiltinToolStore, tenantCfgs store.BuiltinToolTenantConfigStore, tenantStore store.TenantStore, msgBus *bus.MessageBus) *BuiltinToolsHandler {
return &BuiltinToolsHandler{store: s, tenantCfgStore: tenantCfgs, tenantStore: tenantStore, msgBus: msgBus}
}
// RegisterRoutes registers all built-in tool routes on the given mux.
@@ -144,17 +145,16 @@ func (h *BuiltinToolsHandler) handleSetTenantConfig(w http.ResponseWriter, r *ht
writeJSON(w, http.StatusNotImplemented, map[string]string{"error": "tenant config not available"})
return
}
name := r.PathValue("name")
tid := store.TenantIDFromContext(r.Context())
if tid == uuid.Nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "tenant context required"})
if !requireTenantAdmin(w, r, h.tenantStore) {
return
}
name := r.PathValue("name")
tid := store.TenantIDFromContext(r.Context())
var body struct {
Enabled bool `json:"enabled"`
}
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<10)).Decode(&body); err != nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "invalid request body"})
return
}
@@ -165,6 +165,7 @@ func (h *BuiltinToolsHandler) handleSetTenantConfig(w http.ResponseWriter, r *ht
return
}
emitAudit(h.msgBus, r, "builtin_tool.tenant_config.set", "builtin_tool", name)
writeJSON(w, http.StatusOK, map[string]string{"status": "updated"})
}
@@ -174,17 +175,17 @@ func (h *BuiltinToolsHandler) handleDeleteTenantConfig(w http.ResponseWriter, r
writeJSON(w, http.StatusNotImplemented, map[string]string{"error": "tenant config not available"})
return
}
name := r.PathValue("name")
tid := store.TenantIDFromContext(r.Context())
if tid == uuid.Nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "tenant context required"})
if !requireTenantAdmin(w, r, h.tenantStore) {
return
}
name := r.PathValue("name")
tid := store.TenantIDFromContext(r.Context())
if err := h.tenantCfgStore.Delete(r.Context(), tid, name); err != nil {
writeJSON(w, http.StatusInternalServerError, map[string]string{"error": err.Error()})
return
}
emitAudit(h.msgBus, r, "builtin_tool.tenant_config.deleted", "builtin_tool", name)
writeJSON(w, http.StatusOK, map[string]string{"status": "deleted"})
}
internal/http/skills.go
+15 -13
View File
@@ -30,11 +30,12 @@ type SkillsHandler struct {
bundledDir string // original bundled skills dir (fallback for broken managed copies)
msgBus *bus.MessageBus
tenantCfgStore store.SkillTenantConfigStore
tenantStore store.TenantStore
}
// NewSkillsHandler creates a handler for skill management endpoints.
func NewSkillsHandler(skills *pg.PGSkillStore, baseDir, dataDir, bundledDir string, msgBus *bus.MessageBus, tenantCfgStore store.SkillTenantConfigStore) *SkillsHandler {
return &SkillsHandler{skills: skills, baseDir: baseDir, dataDir: dataDir, bundledDir: bundledDir, msgBus: msgBus, tenantCfgStore: tenantCfgStore}
func NewSkillsHandler(skills *pg.PGSkillStore, baseDir, dataDir, bundledDir string, msgBus *bus.MessageBus, tenantCfgStore store.SkillTenantConfigStore, tenantStore store.TenantStore) *SkillsHandler {
return &SkillsHandler{skills: skills, baseDir: baseDir, dataDir: dataDir, bundledDir: bundledDir, msgBus: msgBus, tenantCfgStore: tenantCfgStore, tenantStore: tenantStore}
}
// tenantSkillsDir returns the skills-store directory scoped to the requesting tenant.
@@ -78,8 +79,9 @@ func (h *SkillsHandler) RegisterRoutes(mux *http.ServeMux) {
mux.HandleFunc("POST /v1/skills/install-dep", h.adminMiddleware(h.handleInstallDep))
mux.HandleFunc("GET /v1/skills/runtimes", h.adminMiddleware(h.handleRuntimes))
mux.HandleFunc("POST /v1/skills/{id}/toggle", h.adminMiddleware(h.handleToggle))
mux.HandleFunc("PUT /v1/skills/{id}/tenant-config", h.adminMiddleware(h.handleSetTenantConfig))
mux.HandleFunc("DELETE /v1/skills/{id}/tenant-config", h.adminMiddleware(h.handleDeleteTenantConfig))
// Per-tenant overrides: tenant-level admin check inside handler (not system admin).
mux.HandleFunc("PUT /v1/skills/{id}/tenant-config", h.authMiddleware(h.handleSetTenantConfig))
mux.HandleFunc("DELETE /v1/skills/{id}/tenant-config", h.authMiddleware(h.handleDeleteTenantConfig))
}
func (h *SkillsHandler) authMiddleware(next http.HandlerFunc) http.HandlerFunc {
@@ -525,12 +527,11 @@ func (h *SkillsHandler) handleSetTenantConfig(w http.ResponseWriter, r *http.Req
writeJSON(w, http.StatusNotImplemented, map[string]string{"error": "tenant config not available"})
return
}
locale := store.LocaleFromContext(r.Context())
tid := store.TenantIDFromContext(r.Context())
if tid == uuid.Nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "tenant context required"})
if !requireTenantAdmin(w, r, h.tenantStore) {
return
}
locale := store.LocaleFromContext(r.Context())
tid := store.TenantIDFromContext(r.Context())
idStr := r.PathValue("id")
skillID, err := uuid.Parse(idStr)
@@ -542,7 +543,7 @@ func (h *SkillsHandler) handleSetTenantConfig(w http.ResponseWriter, r *http.Req
var body struct {
Enabled bool `json:"enabled"`
}
if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
if err := json.NewDecoder(http.MaxBytesReader(w, r.Body, 1<<10)).Decode(&body); err != nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": i18n.T(locale, i18n.MsgInvalidJSON)})
return
}
@@ -553,6 +554,7 @@ func (h *SkillsHandler) handleSetTenantConfig(w http.ResponseWriter, r *http.Req
return
}
emitAudit(h.msgBus, r, "skill.tenant_config.set", "skill", idStr)
writeJSON(w, http.StatusOK, map[string]string{"status": "updated"})
}
@@ -562,12 +564,11 @@ func (h *SkillsHandler) handleDeleteTenantConfig(w http.ResponseWriter, r *http.
writeJSON(w, http.StatusNotImplemented, map[string]string{"error": "tenant config not available"})
return
}
locale := store.LocaleFromContext(r.Context())
tid := store.TenantIDFromContext(r.Context())
if tid == uuid.Nil {
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "tenant context required"})
if !requireTenantAdmin(w, r, h.tenantStore) {
return
}
locale := store.LocaleFromContext(r.Context())
tid := store.TenantIDFromContext(r.Context())
idStr := r.PathValue("id")
skillID, err := uuid.Parse(idStr)
@@ -582,5 +583,6 @@ func (h *SkillsHandler) handleDeleteTenantConfig(w http.ResponseWriter, r *http.
return
}
emitAudit(h.msgBus, r, "skill.tenant_config.deleted", "skill", idStr)
writeJSON(w, http.StatusOK, map[string]string{"status": "deleted"})
}
internal/http/tenant_auth_helpers.go
+49
View File
@@ -0,0 +1,49 @@
package http
import (
"net/http"
"github.com/google/uuid"
"github.com/nextlevelbuilder/goclaw/internal/i18n"
"github.com/nextlevelbuilder/goclaw/internal/store"
)
// requireTenantAdmin verifies the caller has owner or admin role within the
// specified tenant. System-wide owners (IsOwnerRole) bypass the check.
// Returns true if authorized, false if an error response was written.
func requireTenantAdmin(w http.ResponseWriter, r *http.Request, ts store.TenantStore) bool {
ctx := r.Context()
// System-wide owner bypasses tenant membership check.
if store.IsOwnerRole(ctx) {
return true
}
if ts == nil {
writeJSON(w, http.StatusNotImplemented, map[string]string{"error": "tenant store not available"})
return false
}
tid := store.TenantIDFromContext(ctx)
if tid == uuid.Nil {
locale := store.LocaleFromContext(ctx)
writeJSON(w, http.StatusBadRequest, map[string]string{
"error": i18n.T(locale, i18n.MsgPermissionDenied, "tenant config"),
})
return false
}
userID := store.UserIDFromContext(ctx)
// GetUserRole returns ("", nil) when the user has no membership in this tenant,
// which correctly falls through to the role check below (denied).
role, err := ts.GetUserRole(ctx, tid, userID)
if err != nil || (role != store.TenantRoleOwner && role != store.TenantRoleAdmin) {
locale := store.LocaleFromContext(ctx)
writeJSON(w, http.StatusForbidden, map[string]string{
"error": i18n.T(locale, i18n.MsgPermissionDenied, "tenant config"),
})
return false
}
return true
}