fix(security): reject cross-module document-access grants (CE0148 guard)

.claude/skills/fix-issue.md

@@ -24,6 +24,7 @@ to the symptom table below, so the next similar issue costs fewer reads.
 | Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker |
 | `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` |
 | CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics |
+| `grant view on page` / `grant execute on microflow\|nanoflow` / `grant access on odata\|published rest service` to a role from **another module** passes `mxcli check`/`exec` but fails the Mendix build with **CE0148 "reselect roles"** — the own-module role works | Document access (page/microflow/nanoflow/service `AllowedModuleRoles`) may only reference the document's **own** module roles; Studio Pro's picker only offers those. The grant path wrote `role.Module + "." + role.Name` verbatim with no same-module check (only `validateModuleRole` = role-exists-in-its-module), so a cross-module reference reached the model. The MOVE path already guarded this (`remapDocumentAccessRoles`) — GRANT didn't | `mdl/executor/cmd_security_defaults.go` (`checkDocumentAccessRolesSameModule`) + the 5 grant handlers in `mdl/executor/cmd_security_write.go` | Pre-check each grant: reject when any `role.Module != docModule` with an actionable message (name the doc's module + suggest the own-module role). Reject (don't silently remap) — a GRANT is explicit, so a wrong role/doc shouldn't be substituted. Wired into page/microflow/nanoflow/OData/published-REST grants. Repro `mdl-examples/bug-tests/ce0148-cross-module-grant.mdl` |
 | CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` |
 | Parser returns `nil` for a known BSON type | Unhandled `default` in a `parseXxx()` switch | `sdk/mpr/parser_microflow.go` or `parser_page.go` | Find the switch by grepping for `default: return nil`; add the missing case |
 | MDL check gives "unexpected token" on valid-looking syntax | Grammar missing rule or token | `mdl/grammar/MDLParser.g4` + `MDLLexer.g4` | Add rule/token, run `make grammar` |

mdl-examples/bug-tests/ce0148-cross-module-grant.mdl

@@ -0,0 +1,46 @@
+-- @version: 10.0+
+-- ============================================================================
+-- CE0148 — cross-module document-access grant.
+--
+-- Document access (page / microflow / nanoflow / published OData|REST service
+-- AllowedModuleRoles) may only reference the document's OWN module roles.
+-- Studio Pro's role picker only offers the containing module's roles, so a
+-- cross-module reference builds with CE0148 "reselect roles" — even though the
+-- reference is otherwise well-formed.
+--
+-- Before the fix, `grant view on page A.Page to B.Role` passed `mxcli check`
+-- and `exec` (validateModuleRole only checks the role exists in ITS module),
+-- then failed the Mendix build with an opaque CE0148. The grant path now
+-- pre-rejects a cross-module role with an actionable error.
+--
+-- Fix: checkDocumentAccessRolesSameModule (mdl/executor/cmd_security_defaults.go)
+-- wired into the 5 grant handlers in mdl/executor/cmd_security_write.go.
+-- ============================================================================
+
+create module Alpha;
+create module Beta;
+
+create module role Alpha.User    description 'Alpha module user';
+create module role Beta.Manager   description 'Beta module manager';
+
+create page Alpha.Overview (
+  title: 'Overview'
+) {
+  layoutgrid g1 {
+    row r1 {
+      column c1 (desktopwidth: 12) {
+        dynamictext txt1 (content: 'Overview', rendermode: H2)
+      }
+    }
+  }
+}
+
+-- CORRECT: grant the page its OWN module's role. Passes check + build.
+grant view on page Alpha.Overview to Alpha.User;
+
+-- WRONG (do NOT uncomment): a cross-module role is rejected by the CE0148 guard.
+-- The build would otherwise fail with CE0148 "reselect roles".
+--   grant view on page Alpha.Overview to Beta.Manager;
+--
+-- To give a Beta-scoped user access, add an Alpha module role and map the Beta
+-- user role to it, then grant the Alpha role above.

mdl/executor/cmd_security_defaults.go

@@ -6,10 +6,40 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	mdlerrors "github.com/mendixlabs/mxcli/mdl/errors"
 	"github.com/mendixlabs/mxcli/model"
 	"github.com/mendixlabs/mxcli/sdk/security"
 )
 
+// checkDocumentAccessRolesSameModule rejects granting a document (page,
+// microflow, nanoflow, or a published OData/REST service) access to a module role
+// from a different module than the document. Mendix stores document access as
+// references to the document's OWN module roles only — Studio Pro's role picker
+// only offers the containing module's roles — so a cross-module reference builds
+// with CE0148 ("reselect roles") even though it is otherwise well-formed. Without
+// this guard the grant writes an invalid reference that passes mxcli but fails the
+// Mendix build with an opaque error. The MOVE path enforces the same rule by
+// remapping to same-named target-module roles (see remapDocumentAccessRoles); on
+// an explicit GRANT we reject instead, so a wrong role or wrong document is not
+// silently substituted.
+//
+// docKind/docName are used only for the message; docModule is the document's module.
+func checkDocumentAccessRolesSameModule(docKind, docModule, docName string, roles []ast.QualifiedName) error {
+	for _, role := range roles {
+		if role.Module != docModule {
+			return mdlerrors.NewValidationf(
+				"cannot grant %s %s.%s access to %s.%s: a %s can only reference module roles from its own module %q "+
+					"(Mendix rejects a cross-module document role with CE0148 \"reselect roles\"). "+
+					"Grant a %s module role instead — e.g. %s.%s if it exists — and map the user role to it.",
+				docKind, docModule, docName, role.Module, role.Name,
+				docKind, docModule,
+				docModule, docModule, role.Name)
+		}
+	}
+	return nil
+}
+
 const (
 	autoDocumentRoleName        = "User"
 	autoDocumentRoleDescription = "Auto-created default role for mxcli document access"

mdl/executor/cmd_security_grant_crossmodule_test.go

@@ -0,0 +1,147 @@
+// SPDX-License-Identifier: Apache-2.0
+
+package executor
+
+import (
+	"testing"
+
+	"github.com/mendixlabs/mxcli/mdl/ast"
+	"github.com/mendixlabs/mxcli/mdl/backend/mock"
+	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
+	"github.com/mendixlabs/mxcli/sdk/pages"
+	"github.com/mendixlabs/mxcli/sdk/security"
+)
+
+// checkDocumentAccessRolesSameModule is the shared CE0148 guard. Test both
+// branches directly — it is pure, so no backend wiring is needed.
+func TestCheckDocumentAccessRolesSameModule(t *testing.T) {
+	sameModule := []ast.QualifiedName{{Module: "Sales", Name: "User"}, {Module: "Sales", Name: "Admin"}}
+	if err := checkDocumentAccessRolesSameModule("page", "Sales", "Overview", sameModule); err != nil {
+		t.Errorf("same-module roles should be accepted, got: %v", err)
+	}
+
+	crossModule := []ast.QualifiedName{{Module: "Sales", Name: "User"}, {Module: "HR", Name: "Manager"}}
+	err := checkDocumentAccessRolesSameModule("page", "Sales", "Overview", crossModule)
+	if err == nil {
+		t.Fatal("cross-module role should be rejected")
+	}
+	msg := err.Error()
+	for _, want := range []string{"CE0148", "Sales", "HR.Manager", "Overview"} {
+		assertContainsStr(t, msg, want)
+	}
+}
+
+// TestGrantPageAccess_CrossModuleRole_Rejected: granting a page a role from
+// another module must fail with the CE0148 guard, before any write.
+func TestGrantPageAccess_CrossModuleRole_Rejected(t *testing.T) {
+	mod := mkModule("Sales")
+	h := mkHierarchy(mod)
+	pg := mkPage(mod.ID, "Overview")
+
+	updated := false
+	mb := &mock.MockBackend{
+		IsConnectedFunc: func() bool { return true },
+		ListPagesFunc:   func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil },
+		UpdateAllowedRolesFunc: func(unitID model.ID, roles []string) error {
+			updated = true
+			return nil
+		},
+	}
+	ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
+
+	err := execGrantPageAccess(ctx, &ast.GrantPageAccessStmt{
+		Page:  ast.QualifiedName{Module: "Sales", Name: "Overview"},
+		Roles: []ast.QualifiedName{{Module: "HR", Name: "Manager"}},
+	})
+	assertError(t, err)
+	assertContainsStr(t, err.Error(), "CE0148")
+	assertContainsStr(t, err.Error(), "HR.Manager")
+	if updated {
+		t.Error("UpdateAllowedRoles must not be called when the grant is rejected")
+	}
+}
+
+// TestGrantPageAccess_SameModuleRole_Succeeds is the regression guard: a
+// same-module grant still works and writes the role.
+func TestGrantPageAccess_SameModuleRole_Succeeds(t *testing.T) {
+	mod := mkModule("Sales")
+	h := mkHierarchy(mod)
+	pg := mkPage(mod.ID, "Overview")
+
+	var wrote []string
+	mb := &mock.MockBackend{
+		IsConnectedFunc: func() bool { return true },
+		ListPagesFunc:   func() ([]*pages.Page, error) { return []*pages.Page{pg}, nil },
+		ListModulesFunc: func() ([]*model.Module, error) { return []*model.Module{mod}, nil },
+		GetModuleSecurityFunc: func(moduleID model.ID) (*security.ModuleSecurity, error) {
+			return &security.ModuleSecurity{ModuleRoles: []*security.ModuleRole{{Name: "User"}}}, nil
+		},
+		UpdateAllowedRolesFunc: func(unitID model.ID, roles []string) error { wrote = roles; return nil },
+	}
+	ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
+
+	err := execGrantPageAccess(ctx, &ast.GrantPageAccessStmt{
+		Page:  ast.QualifiedName{Module: "Sales", Name: "Overview"},
+		Roles: []ast.QualifiedName{{Module: "Sales", Name: "User"}},
+	})
+	assertNoError(t, err)
+	found := false
+	for _, r := range wrote {
+		if r == "Sales.User" {
+			found = true
+		}
+	}
+	if !found {
+		t.Errorf("expected Sales.User in written roles, got %v", wrote)
+	}
+}
+
+// TestGrantMicroflowAccess_CrossModuleRole_Rejected mirrors the page case for a
+// microflow (the guard is wired identically across all document types).
+func TestGrantMicroflowAccess_CrossModuleRole_Rejected(t *testing.T) {
+	mod := mkModule("Sales")
+	h := mkHierarchy(mod)
+	mf := mkMicroflow(mod.ID, "ACT_Do")
+
+	mb := &mock.MockBackend{
+		IsConnectedFunc:    func() bool { return true },
+		ListMicroflowsFunc: func() ([]*microflows.Microflow, error) { return []*microflows.Microflow{mf}, nil },
+		UpdateAllowedRolesFunc: func(unitID model.ID, roles []string) error {
+			t.Error("UpdateAllowedRoles must not be called for a rejected cross-module grant")
+			return nil
+		},
+	}
+	ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
+
+	err := execGrantMicroflowAccess(ctx, &ast.GrantMicroflowAccessStmt{
+		Microflow: ast.QualifiedName{Module: "Sales", Name: "ACT_Do"},
+		Roles:     []ast.QualifiedName{{Module: "HR", Name: "Manager"}},
+	})
+	assertError(t, err)
+	assertContainsStr(t, err.Error(), "CE0148")
+}
+
+// TestGrantNanoflowAccess_CrossModuleRole_Rejected mirrors the case for a nanoflow.
+func TestGrantNanoflowAccess_CrossModuleRole_Rejected(t *testing.T) {
+	mod := mkModule("Sales")
+	h := mkHierarchy(mod)
+	nf := mkNanoflow(mod.ID, "ACT_Nano")
+
+	mb := &mock.MockBackend{
+		IsConnectedFunc:   func() bool { return true },
+		ListNanoflowsFunc: func() ([]*microflows.Nanoflow, error) { return []*microflows.Nanoflow{nf}, nil },
+		UpdateAllowedRolesFunc: func(unitID model.ID, roles []string) error {
+			t.Error("UpdateAllowedRoles must not be called for a rejected cross-module grant")
+			return nil
+		},
+	}
+	ctx, _ := newMockCtx(t, withBackend(mb), withHierarchy(h))
+
+	err := execGrantNanoflowAccess(ctx, &ast.GrantNanoflowAccessStmt{
+		Nanoflow: ast.QualifiedName{Module: "Sales", Name: "ACT_Nano"},
+		Roles:    []ast.QualifiedName{{Module: "HR", Name: "Manager"}},
+	})
+	assertError(t, err)
+	assertContainsStr(t, err.Error(), "CE0148")
+}

mdl/executor/cmd_security_write.go

@@ -590,6 +590,11 @@ func execGrantMicroflowAccess(ctx *ExecContext, s *ast.GrantMicroflowAccessStmt)
 			continue
 		}
 
+		// Reject cross-module role grants before they reach the model (CE0148 guard).
+		if err := checkDocumentAccessRolesSameModule("microflow", modName, mf.Name, s.Roles); err != nil {
+			return err
+		}
+
 		// Validate all roles exist
 		for _, role := range s.Roles {
 			if err := validateModuleRole(ctx, role); err != nil {
@@ -707,6 +712,11 @@ func execGrantNanoflowAccess(ctx *ExecContext, s *ast.GrantNanoflowAccessStmt) e
 			continue
 		}
 
+		// Reject cross-module role grants before they reach the model (CE0148 guard).
+		if err := checkDocumentAccessRolesSameModule("nanoflow", modName, nf.Name, s.Roles); err != nil {
+			return err
+		}
+
 		for _, role := range s.Roles {
 			if err := validateModuleRole(ctx, role); err != nil {
 				return err
@@ -820,6 +830,11 @@ func execGrantPageAccess(ctx *ExecContext, s *ast.GrantPageAccessStmt) error {
 			continue
 		}
 
+		// Reject cross-module role grants before they reach the model (CE0148 guard).
+		if err := checkDocumentAccessRolesSameModule("page", modName, pg.Name, s.Roles); err != nil {
+			return err
+		}
+
 		// Validate all roles exist
 		for _, role := range s.Roles {
 			if err := validateModuleRole(ctx, role); err != nil {
@@ -1165,6 +1180,11 @@ func execGrantODataServiceAccess(ctx *ExecContext, s *ast.GrantODataServiceAcces
 			continue
 		}
 
+		// Reject cross-module role grants before they reach the model (CE0148 guard).
+		if err := checkDocumentAccessRolesSameModule("OData service", modName, svc.Name, s.Roles); err != nil {
+			return err
+		}
+
 		// Validate all roles exist
 		for _, role := range s.Roles {
 			if err := validateModuleRole(ctx, role); err != nil {
@@ -1292,6 +1312,11 @@ func execGrantPublishedRestServiceAccess(ctx *ExecContext, s *ast.GrantPublished
 			continue
 		}
 
+		// Reject cross-module role grants before they reach the model (CE0148 guard).
+		if err := checkDocumentAccessRolesSameModule("published REST service", modName, svc.Name, s.Roles); err != nil {
+			return err
+		}
+
 		// Validate all roles exist
 		for _, role := range s.Roles {
 			if err := validateModuleRole(ctx, role); err != nil {