From d82ad782c2a73a52c329b6345a8cc28213e532dd Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 24 Jul 2026 12:58:12 +0000 Subject: [PATCH] fix(security): reject cross-module document-access grants (CE0148 guard) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Granting a page/microflow/nanoflow/published OData|REST service access to a module role from a DIFFERENT module than the document passed `mxcli check` and `exec`, then failed the Mendix build with an opaque CE0148 "reselect roles". Document access (AllowedModuleRoles) may only reference the document's OWN module roles — Studio Pro's role picker only offers those — so a cross-module reference is invalid even though it is well-formed. The grant handlers wrote `role.Module + "." + role.Name` verbatim, guarded only by validateModuleRole (which checks the role exists in ITS module, not that it matches the document's module). The MOVE path already enforced the same-module rule by remapping (remapDocumentAccessRoles); GRANT had no equivalent. Add checkDocumentAccessRolesSameModule and wire it into all five grant handlers (page, microflow, nanoflow, OData service, published REST service). It rejects a cross-module role with an actionable message that names the document's module and suggests the own-module role — turning an opaque deploy-time failure into an immediate, clear error. Reject (not silently remap): a GRANT is explicit, so a wrong role or wrong document must not be substituted. Tests: the pure guard (both branches) plus page/microflow/nanoflow handler tests via MockBackend (cross-module rejected, no write; same-module still works). Adds mdl-examples/bug-tests/ce0148-cross-module-grant.mdl and a fix-issue.md symptom-table entry. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01JXnEgoc2NQP1Y2TWMCMXC4 --- .claude/skills/fix-issue.md | 1 + .../bug-tests/ce0148-cross-module-grant.mdl | 46 ++++++ mdl/executor/cmd_security_defaults.go | 30 ++++ .../cmd_security_grant_crossmodule_test.go | 147 ++++++++++++++++++ mdl/executor/cmd_security_write.go | 25 +++ 5 files changed, 249 insertions(+) create mode 100644 mdl-examples/bug-tests/ce0148-cross-module-grant.mdl create mode 100644 mdl/executor/cmd_security_grant_crossmodule_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index adfa3c8dd..7d911efbc 100644 --- a/.claude/skills/fix-issue.md +++ b/.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` | diff --git a/mdl-examples/bug-tests/ce0148-cross-module-grant.mdl b/mdl-examples/bug-tests/ce0148-cross-module-grant.mdl new file mode 100644 index 000000000..daf2c3333 --- /dev/null +++ b/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. diff --git a/mdl/executor/cmd_security_defaults.go b/mdl/executor/cmd_security_defaults.go index 4659e99f4..160924727 100644 --- a/mdl/executor/cmd_security_defaults.go +++ b/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" diff --git a/mdl/executor/cmd_security_grant_crossmodule_test.go b/mdl/executor/cmd_security_grant_crossmodule_test.go new file mode 100644 index 000000000..e7ec370ea --- /dev/null +++ b/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") +} diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index 4a77fd9b0..beb415345 100644 --- a/mdl/executor/cmd_security_write.go +++ b/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 {