diff --git a/internal/gateway/methods/config.go b/internal/gateway/methods/config.go index 91fdbae3..6fd07206 100644 --- a/internal/gateway/methods/config.go +++ b/internal/gateway/methods/config.go @@ -5,6 +5,7 @@ import ( "encoding/json" "log/slog" + "github.com/google/uuid" "github.com/titanous/json5" "github.com/nextlevelbuilder/goclaw/internal/bus" @@ -36,10 +37,10 @@ func (m *ConfigMethods) SetSystemConfigSync(fn func(ctx context.Context, cfg *co } func (m *ConfigMethods) Register(router *gateway.MethodRouter) { - router.Register(protocol.MethodConfigGet, m.requireOwner(m.handleGet)) - router.Register(protocol.MethodConfigApply, m.requireOwner(m.handleApply)) - router.Register(protocol.MethodConfigPatch, m.requireOwner(m.handlePatch)) - router.Register(protocol.MethodConfigSchema, m.requireOwner(m.handleSchema)) + router.Register(protocol.MethodConfigGet, m.requireMasterScope(m.requireOwner(m.handleGet))) + router.Register(protocol.MethodConfigApply, m.requireMasterScope(m.requireOwner(m.handleApply))) + router.Register(protocol.MethodConfigPatch, m.requireMasterScope(m.requireOwner(m.handlePatch))) + router.Register(protocol.MethodConfigSchema, m.requireMasterScope(m.requireOwner(m.handleSchema))) } // requireOwner wraps a handler to only allow owner-role users. @@ -57,6 +58,40 @@ func (m *ConfigMethods) requireOwner(next gateway.MethodHandler) gateway.MethodH } } +// requireMasterScope rejects config.* calls when the caller's ctx is scoped to +// a non-master tenant. System owner callers (bypass-all) are allowed through. +// +// Background: config.* mutates the master in-memory *config.Config and the +// on-disk config.json. A non-master tenant admin calling config.patch would +// corrupt master state + leak master config to other tenants. This guard keeps +// config.* strictly master-scoped until a tenant-aware refactor lands. +func (m *ConfigMethods) requireMasterScope(next gateway.MethodHandler) gateway.MethodHandler { + return func(ctx context.Context, client *gateway.Client, req *protocol.RequestFrame) { + if !isMasterScopeContext(ctx) { + locale := store.LocaleFromContext(ctx) + client.SendResponse(protocol.NewErrorResponse( + req.ID, + protocol.ErrUnauthorized, + i18n.T(locale, i18n.MsgConfigMasterScopeOnly), + )) + return + } + next(ctx, client, req) + } +} + +// isMasterScopeContext returns true when ctx should be treated as master-scope: +// (a) system owner role (bypass-all), or +// (b) tenant id is unset (uuid.Nil — legacy / system callers), or +// (c) tenant id equals store.MasterTenantID. +func isMasterScopeContext(ctx context.Context) bool { + if store.IsOwnerRole(ctx) { + return true + } + tid := store.TenantIDFromContext(ctx) + return tid == uuid.Nil || tid == store.MasterTenantID +} + func (m *ConfigMethods) handleGet(_ context.Context, client *gateway.Client, req *protocol.RequestFrame) { client.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{ "config": m.cfg.MaskedCopy(), diff --git a/internal/gateway/methods/config_tenant_guard_test.go b/internal/gateway/methods/config_tenant_guard_test.go new file mode 100644 index 00000000..5aa003cd --- /dev/null +++ b/internal/gateway/methods/config_tenant_guard_test.go @@ -0,0 +1,157 @@ +package methods + +import ( + "context" + "testing" + + "github.com/google/uuid" + + "github.com/nextlevelbuilder/goclaw/internal/gateway" + "github.com/nextlevelbuilder/goclaw/internal/store" + "github.com/nextlevelbuilder/goclaw/pkg/protocol" +) + +// ---- Tests: isMasterScopeContext helper ---- + +func TestIsMasterScopeContext_OwnerRole_Allowed(t *testing.T) { + ctx := store.WithRole(context.Background(), store.RoleOwner) + // Tenant set to a non-master tenant — owner role must still pass + ctx = store.WithTenantID(ctx, uuid.MustParse("11111111-1111-1111-1111-111111111111")) + if !isMasterScopeContext(ctx) { + t.Fatalf("owner role with non-master tenant should be allowed") + } +} + +func TestIsMasterScopeContext_NilTenant_Allowed(t *testing.T) { + // Legacy / system callers without tenant scope → treated as master + ctx := context.Background() + if !isMasterScopeContext(ctx) { + t.Fatalf("nil tenant ctx should be allowed (master-scope fallback)") + } +} + +func TestIsMasterScopeContext_MasterTenant_Allowed(t *testing.T) { + ctx := store.WithTenantID(context.Background(), store.MasterTenantID) + if !isMasterScopeContext(ctx) { + t.Fatalf("master tenant ctx should be allowed") + } +} + +func TestIsMasterScopeContext_NonMasterTenantNoOwner_Denied(t *testing.T) { + tid := uuid.MustParse("22222222-2222-2222-2222-222222222222") + ctx := store.WithTenantID(context.Background(), tid) + if isMasterScopeContext(ctx) { + t.Fatalf("non-master tenant ctx without owner role must be denied") + } +} + +func TestIsMasterScopeContext_NonMasterTenantWithOwnerRole_Allowed(t *testing.T) { + // A system owner visiting a tenant dashboard — bypass-all allows through + tid := uuid.MustParse("33333333-3333-3333-3333-333333333333") + ctx := store.WithTenantID(context.Background(), tid) + ctx = store.WithRole(ctx, store.RoleOwner) + if !isMasterScopeContext(ctx) { + t.Fatalf("owner role must bypass tenant scope check") + } +} + +// ---- Tests: requireMasterScope middleware ---- + +func configGuardRequest(method string) *protocol.RequestFrame { + return &protocol.RequestFrame{ + Type: protocol.FrameTypeRequest, + ID: "cfg-guard-req-1", + Method: method, + } +} + +// nextCalledHandler returns a handler that flips *called to true when invoked. +func nextCalledHandler(called *bool) gateway.MethodHandler { + return func(_ context.Context, _ *gateway.Client, _ *protocol.RequestFrame) { + *called = true + } +} + +func TestRequireMasterScope_MasterTenant_CallsNext(t *testing.T) { + m := &ConfigMethods{} + var called bool + h := m.requireMasterScope(nextCalledHandler(&called)) + + ctx := store.WithTenantID(context.Background(), store.MasterTenantID) + h(ctx, nullClient(), configGuardRequest(protocol.MethodConfigPatch)) + + if !called { + t.Fatalf("expected next handler to be called for master tenant ctx") + } +} + +func TestRequireMasterScope_NilTenant_CallsNext(t *testing.T) { + m := &ConfigMethods{} + var called bool + h := m.requireMasterScope(nextCalledHandler(&called)) + + h(context.Background(), nullClient(), configGuardRequest(protocol.MethodConfigGet)) + + if !called { + t.Fatalf("expected next handler to be called for nil tenant ctx (legacy compat)") + } +} + +func TestRequireMasterScope_OwnerRole_CallsNext(t *testing.T) { + m := &ConfigMethods{} + var called bool + h := m.requireMasterScope(nextCalledHandler(&called)) + + // System owner visiting a non-master tenant ctx — must pass + ctx := store.WithTenantID(context.Background(), uuid.MustParse("44444444-4444-4444-4444-444444444444")) + ctx = store.WithRole(ctx, store.RoleOwner) + h(ctx, nullClient(), configGuardRequest(protocol.MethodConfigApply)) + + if !called { + t.Fatalf("expected next handler to be called for owner role bypass") + } +} + +func TestRequireMasterScope_NonMasterTenantNoOwner_BlocksNext(t *testing.T) { + m := &ConfigMethods{} + var called bool + h := m.requireMasterScope(nextCalledHandler(&called)) + + // Non-master tenant admin (non-owner role) — must be blocked + ctx := store.WithTenantID(context.Background(), uuid.MustParse("55555555-5555-5555-5555-555555555555")) + ctx = store.WithRole(ctx, "admin") + h(ctx, nullClient(), configGuardRequest(protocol.MethodConfigPatch)) + + if called { + t.Fatalf("non-master tenant non-owner ctx must NOT reach next handler") + } +} + +// ---- Test: middleware chain (requireMasterScope → requireOwner → handler) ---- + +// TestRequireMasterScope_ChainedWithRequireOwner asserts the master-scope guard +// fires BEFORE requireOwner (which uses client.IsOwner()). Non-master tenant ctx +// must be rejected even though nullClient has no owner role set — this protects +// the downstream handler from ever touching m.cfg for a non-master caller. +func TestRequireMasterScope_ChainedWithRequireOwner(t *testing.T) { + m := &ConfigMethods{} + var innerCalled bool + inner := nextCalledHandler(&innerCalled) + chain := m.requireMasterScope(m.requireOwner(inner)) + + // Non-master tenant ctx → master-scope guard rejects first + ctx := store.WithTenantID(context.Background(), uuid.MustParse("66666666-6666-6666-6666-666666666666")) + ctx = store.WithRole(ctx, "admin") + + // Must not panic (handler with nil m.cfg is never reached) + defer func() { + if r := recover(); r != nil { + t.Fatalf("chained middleware panicked (guard did not fire early): %v", r) + } + }() + chain(ctx, nullClient(), configGuardRequest(protocol.MethodConfigPatch)) + + if innerCalled { + t.Fatalf("inner handler must not be reached when master-scope guard rejects") + } +} diff --git a/internal/i18n/catalog_en.go b/internal/i18n/catalog_en.go index fb39ebb1..fd6e33c4 100644 --- a/internal/i18n/catalog_en.go +++ b/internal/i18n/catalog_en.go @@ -105,8 +105,9 @@ func init() { MsgInvalidLogAction: "action must be 'start' or 'stop'", // Config - MsgRawConfigRequired: "raw config is required", - MsgRawPatchRequired: "raw patch is required", + MsgRawConfigRequired: "raw config is required", + MsgRawPatchRequired: "raw patch is required", + MsgConfigMasterScopeOnly: "config.* methods are master-scope only; use tenant tool config endpoints for per-tenant overrides", // Storage / File MsgCannotDeleteSkillsDir: "cannot delete skills directories", diff --git a/internal/i18n/catalog_vi.go b/internal/i18n/catalog_vi.go index ae9f611f..1f2227b0 100644 --- a/internal/i18n/catalog_vi.go +++ b/internal/i18n/catalog_vi.go @@ -105,8 +105,9 @@ func init() { MsgInvalidLogAction: "action phải là 'start' hoặc 'stop'", // Config - MsgRawConfigRequired: "cấu hình raw là bắt buộc", - MsgRawPatchRequired: "patch raw là bắt buộc", + MsgRawConfigRequired: "cấu hình raw là bắt buộc", + MsgRawPatchRequired: "patch raw là bắt buộc", + MsgConfigMasterScopeOnly: "config.* chỉ áp dụng cho master scope; dùng endpoint tenant tool config cho override theo tenant", // Storage / File MsgCannotDeleteSkillsDir: "không thể xóa thư mục skill", diff --git a/internal/i18n/catalog_zh.go b/internal/i18n/catalog_zh.go index a13d09a5..61ddfa7d 100644 --- a/internal/i18n/catalog_zh.go +++ b/internal/i18n/catalog_zh.go @@ -105,8 +105,9 @@ func init() { MsgInvalidLogAction: "action 必须是 'start' 或 'stop'", // Config - MsgRawConfigRequired: "raw 配置是必填项", - MsgRawPatchRequired: "raw 补丁是必填项", + MsgRawConfigRequired: "raw 配置是必填项", + MsgRawPatchRequired: "raw 补丁是必填项", + MsgConfigMasterScopeOnly: "config.* 方法仅适用于主作用域；使用租户工具配置端点进行租户级覆盖", // Storage / File MsgCannotDeleteSkillsDir: "无法删除Skill目录", diff --git a/internal/i18n/keys.go b/internal/i18n/keys.go index f8ab95f1..a84bd22d 100644 --- a/internal/i18n/keys.go +++ b/internal/i18n/keys.go @@ -106,8 +106,9 @@ const ( MsgInvalidLogAction = "error.invalid_log_action" // "action must be 'start' or 'stop'" // --- Config --- - MsgRawConfigRequired = "error.raw_config_required" // "raw config is required" - MsgRawPatchRequired = "error.raw_patch_required" // "raw patch is required" + MsgRawConfigRequired = "error.raw_config_required" // "raw config is required" + MsgRawPatchRequired = "error.raw_patch_required" // "raw patch is required" + MsgConfigMasterScopeOnly = "error.config_master_scope_only" // "config.* methods are master-scope only" // --- Storage / File --- MsgCannotDeleteSkillsDir = "error.cannot_delete_skills_dir" // "cannot delete skills directories"