diff --git a/internal/gateway/router.go b/internal/gateway/router.go index dd32aafd..974dc2d6 100644 --- a/internal/gateway/router.go +++ b/internal/gateway/router.go @@ -67,7 +67,16 @@ func (r *MethodRouter) Handle(ctx context.Context, client *Client, req *protocol // Permission check: skip for connect, health, and browser pairing status (used by unauthenticated clients) if req.Method != protocol.MethodConnect && req.Method != protocol.MethodHealth && req.Method != protocol.MethodBrowserPairingStatus { if pe := r.server.policyEngine; pe != nil { - if !pe.CanAccess(client.role, req.Method) { + // provisionScopeAllowed implements the narrow method-scoped + // exception for operator.provision credentials (issue #1524, a + // regression from the CVE #866 fail-closed hardening). The tenant + // handlers already admit ScopeProvision on tenants.create and + // tenants.users.add, but the role-only check below maps + // provision-only keys to viewer and rejects them before the + // handler runs. The exception grants exactly those two methods to + // credentials carrying ScopeProvision — no role promotion, and + // every other admin/write surface stays denied. + if !pe.CanAccess(client.role, req.Method) && !provisionScopeAllowed(client, req.Method) { required := permissions.MethodRole(req.Method) slog.Warn("security.permission_denied", "method", req.Method, @@ -121,6 +130,13 @@ func (r *MethodRouter) registerDefaults() { r.Register(protocol.MethodStatus, r.handleStatus) } +// provisionScopeAllowed reports whether a client carrying the operator.provision +// scope may call the given method. True only for the two tenant-provisioning +// RPCs — see permissions.IsProvisionMethod. +func provisionScopeAllowed(c *Client, method string) bool { + return permissions.HasProvisionScope(c.scopes) && permissions.IsProvisionMethod(method) +} + // --- Built-in handlers --- func (r *MethodRouter) handleConnect(ctx context.Context, client *Client, req *protocol.RequestFrame) { diff --git a/internal/gateway/router_test.go b/internal/gateway/router_test.go index f3d3d7fb..cf7ca7cb 100644 --- a/internal/gateway/router_test.go +++ b/internal/gateway/router_test.go @@ -6,6 +6,8 @@ import ( "testing" "time" + "github.com/google/uuid" + "github.com/nextlevelbuilder/goclaw/internal/config" "github.com/nextlevelbuilder/goclaw/internal/permissions" "github.com/nextlevelbuilder/goclaw/pkg/protocol" @@ -62,3 +64,90 @@ func TestHandleConnectAllowsExplicitInsecureNoTokenOptIn(t *testing.T) { t.Fatalf("role = %q, want operator", client.role) } } + +// callMethod dispatches method through the router and returns the response frame. +func callMethod(t *testing.T, server *Server, client *Client, method string) *protocol.ResponseFrame { + t.Helper() + req := &protocol.RequestFrame{ID: "req-" + method, Method: method} + server.router.Handle(context.Background(), client, req) + select { + case raw := <-client.send: + var resp protocol.ResponseFrame + if err := json.Unmarshal(raw, &resp); err != nil { + t.Fatalf("unmarshal response: %v", err) + } + return &resp + case <-time.After(500 * time.Millisecond): + t.Fatalf("expected a response frame for %s", method) + return nil + } +} + +// TestRouterProvisionScopeTenantMethods covers the narrow method-scoped +// exception for operator.provision credentials (issue #1524): provision-only +// API keys are admitted on exactly tenants.create and tenants.users.add — +// the two methods the tenant handlers already gate on ScopeProvision — while +// every other admin/write surface stays denied and plain viewers gain nothing. +func TestRouterProvisionScopeTenantMethods(t *testing.T) { + cfg := config.Default() + cfg.Gateway.Host = "127.0.0.1" + cfg.Gateway.Token = "test-token" + + server := NewServer(cfg, nil, nil, nil) + server.SetPolicyEngine(permissions.NewPolicyEngine(nil)) + + reached := map[string]bool{} + for _, method := range []string{ + protocol.MethodTenantsCreate, + protocol.MethodTenantsUsersAdd, + protocol.MethodTenantsUpdate, + protocol.MethodAgentsCreate, + } { + m := method + server.router.Register(m, func(ctx context.Context, c *Client, req *protocol.RequestFrame) { + reached[m] = true + c.SendResponse(protocol.NewOKResponse(req.ID, map[string]any{"ok": true})) + }) + } + + // Provision-only API key: RoleFromScopes maps it to viewer; scopes carry + // operator.provision (as the WS connect path would set them). + prov, _ := NewCapturingTestClient(permissions.RoleViewer, uuid.Nil, "provisioner", 8) + prov.scopes = []permissions.Scope{permissions.ScopeProvision} + + // 1) Provision-only succeeds on the two tenant-provisioning methods. + for _, method := range []string{protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd} { + resp := callMethod(t, server, prov, method) + if resp.Error != nil { + t.Fatalf("provision-only %s: unexpected error %v (code %s)", method, resp.Error.Message, resp.Error.Code) + } + if !reached[method] { + t.Fatalf("provision-only %s: handler never invoked", method) + } + } + + // 2) Provision-only remains denied on other admin/write surfaces. + for _, method := range []string{protocol.MethodTenantsUpdate, protocol.MethodAgentsCreate} { + resp := callMethod(t, server, prov, method) + if resp.Error == nil || resp.Error.Code != protocol.ErrUnauthorized { + t.Fatalf("provision-only %s: want unauthorized, got error=%v reached=%v", method, resp.Error, reached[method]) + } + } + + // 3) A viewer WITHOUT the provision scope still cannot reach the two + // tenant-provisioning methods. + viewer, _ := NewCapturingTestClient(permissions.RoleViewer, uuid.Nil, "viewer", 8) + for _, method := range []string{protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd} { + resp := callMethod(t, server, viewer, method) + if resp.Error == nil || resp.Error.Code != protocol.ErrUnauthorized { + t.Fatalf("plain viewer %s: want unauthorized, got error=%v reached=%v", method, resp.Error, reached[method]) + } + } + + // 4) An unauthenticated client stays denied. + anon, _ := NewCapturingTestClient("", uuid.Nil, "", 8) + resp := callMethod(t, server, anon, protocol.MethodTenantsCreate) + if resp.Error == nil || resp.Error.Code != protocol.ErrUnauthorized { + t.Fatalf("unauthenticated tenants.create: want unauthorized, got error=%v reached=%v", resp.Error, reached[protocol.MethodTenantsCreate]) + } +} diff --git a/internal/permissions/policy.go b/internal/permissions/policy.go index 8e9aebc4..f6b9ac7c 100644 --- a/internal/permissions/policy.go +++ b/internal/permissions/policy.go @@ -168,6 +168,24 @@ func RoleFromScopes(scopes []Scope) Role { return RoleViewer } +// IsProvisionMethod reports whether method is one of the tenant-provisioning +// RPCs that ScopeProvision exists to grant (issue #1524). ScopeProvision is a +// least-privilege scope for automated tenant onboarding: it admits exactly +// these two methods and nothing else — never a role promotion or a broad +// admin bypass. +func IsProvisionMethod(method string) bool { + switch method { + case protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd: + return true + } + return false +} + +// HasProvisionScope reports whether scopes include ScopeProvision. +func HasProvisionScope(scopes []Scope) bool { + return slices.Contains(scopes, ScopeProvision) +} + // MethodRole returns the minimum role required for a given RPC method. // // Policy is fail-closed (default-deny): methods absent from every allowlist diff --git a/internal/permissions/policy_test.go b/internal/permissions/policy_test.go index 60193831..0239f532 100644 --- a/internal/permissions/policy_test.go +++ b/internal/permissions/policy_test.go @@ -91,6 +91,45 @@ func TestRoleFromScopes(t *testing.T) { } } +// TestIsProvisionMethod covers the ScopeProvision method allowlist (issue #1524): +// exactly tenants.create and tenants.users.add, nothing else. +func TestIsProvisionMethod(t *testing.T) { + allowed := []string{protocol.MethodTenantsCreate, protocol.MethodTenantsUsersAdd} + for _, method := range allowed { + if !IsProvisionMethod(method) { + t.Fatalf("IsProvisionMethod(%q) = false, want true", method) + } + } + denied := []string{ + protocol.MethodTenantsUpdate, + protocol.MethodTenantsUsersRemove, + protocol.MethodAgentsCreate, + protocol.MethodAPIKeysCreate, + protocol.MethodConfigApply, + "tenants.nonexistent", + } + for _, method := range denied { + if IsProvisionMethod(method) { + t.Fatalf("IsProvisionMethod(%q) = true, want false", method) + } + } +} + +func TestHasProvisionScope(t *testing.T) { + if !HasProvisionScope([]Scope{ScopeProvision}) { + t.Fatal("HasProvisionScope([operator.provision]) = false, want true") + } + if !HasProvisionScope([]Scope{ScopeRead, ScopeProvision}) { + t.Fatal("HasProvisionScope([operator.read operator.provision]) = false, want true") + } + if HasProvisionScope(nil) { + t.Fatal("HasProvisionScope(nil) = true, want false") + } + if HasProvisionScope([]Scope{ScopeAdmin}) { + t.Fatal("HasProvisionScope([operator.admin]) = true, want false") + } +} + // --- CanAccess: role-based method access --- func TestCanAccess_AdminMethods(t *testing.T) {