From 058632d3499e37e14a9f7e95341665231a335bdc Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 00:13:36 +0000 Subject: [PATCH 1/6] fix: describe silently dropped trailing XPath constraint groups (#772) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mendix stores sibling predicate groups concatenated in one XPathConstraint field: [Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]] [Status != 'Completed'] [CompletionDate = empty] The grammar's xpathConstraint rule matches ONE bracket group. ParseXPathConstraint removes the error listeners, so ANTLR parsed the first group, left the rest on the token stream, and still returned ok=true. enrichXPathConstraintForDescribe read that as a full parse and re-rendered only what came back — its `if !ok { return original }` fallback never fired — so describe emitted: where Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]; That is worse than a crash. The output looks complete while describing a materially less restrictive query than the project contains, which makes correct defensive code read as buggy — and `describe` is what an agent reads to decide whether code is right. Fixed in two layers: 1. ParseXPathConstraint reports a partial parse as a failure (require the token stream to be at EOF). That alone stops the data loss: the caller falls back to the stored string, which the render path then splits correctly. 2. visitor.SplitXPathPredicateGroups splits a constraint into its top-level groups, and each is enriched and rendered separately — so enum enrichment reaches groups after the first, not just the first. The splitter tracks nesting depth and quoting, because the previous "][" split mangled both a nested [A/B[x = 1]] and a literal containing ']'. The render path now uses it too. Verified end-to-end on a real 11.12.2 project carrying the reported constraint shape: all three groups render, Status is enriched to its qualified enum value in the second group, the output re-parses and re-executes to an identical flow, and `mx check` reports 0 errors. A/B against a pre-fix binary on the same project reproduces the two dropped groups exactly as reported. All three guards mutation-checked. Refs mendixlabs/mxcli#772 --- .claude/skills/fix-issue.md | 1 + .../bug-tests/772-xpath-constraint-groups.mdl | 68 ++++++++++++++++ mdl/executor/cmd_microflows_format_action.go | 64 ++++++++++----- mdl/executor/xpath_enrich_772_test.go | 65 +++++++++++++++ mdl/visitor/visitor_xpath_public.go | 12 +++ mdl/visitor/xpath_groups.go | 79 +++++++++++++++++++ mdl/visitor/xpath_groups_test.go | 79 +++++++++++++++++++ 7 files changed, 347 insertions(+), 21 deletions(-) create mode 100644 mdl-examples/bug-tests/772-xpath-constraint-groups.mdl create mode 100644 mdl/executor/xpath_enrich_772_test.go create mode 100644 mdl/visitor/xpath_groups.go create mode 100644 mdl/visitor/xpath_groups_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index f4b24d120..3b8136797 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -316,6 +316,7 @@ cases for these three BSON types — they fell to `default: return nil`. | `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases | | `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests | | `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | +| `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/mdl-examples/bug-tests/772-xpath-constraint-groups.mdl b/mdl-examples/bug-tests/772-xpath-constraint-groups.mdl new file mode 100644 index 000000000..9655c8fe4 --- /dev/null +++ b/mdl-examples/bug-tests/772-xpath-constraint-groups.mdl @@ -0,0 +1,68 @@ +-- Bug #772: describe silently dropped trailing XPath constraint groups +-- +-- Mendix stores sibling predicate groups concatenated in one XPathConstraint field: +-- +-- [Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]] +-- [Status != 'Completed'] +-- [CompletionDate = empty] +-- +-- The MDL grammar rule matches ONE bracket group. With the error listeners removed, +-- ANTLR parsed the first group, left the rest on the token stream, and still +-- reported success. enrichXPathConstraintForDescribe read that as a full parse and +-- re-rendered only what came back, so describe emitted: +-- +-- where Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]; +-- +-- The last two predicates vanished with no warning. That is worse than a crash: the +-- output looks complete while describing a materially LESS RESTRICTIVE query than +-- the project actually contains, which makes correct defensive code read as buggy. +-- +-- Two fixes, defence in depth: +-- 1. ParseXPathConstraint now reports a partial parse as a failure, so a caller +-- falls back to the stored string instead of silently truncating it. +-- 2. visitor.SplitXPathPredicateGroups splits the constraint into top-level groups +-- — tracking nesting depth and quoting, so a nested [..] or a ']' inside a +-- string literal is handled — and each group is enriched and rendered on its +-- own. Splitting on "][" mangled both of those. +-- +-- Verify: +-- 1. mxcli exec 772-xpath-constraint-groups.mdl -p app.mpr +-- 2. mxcli -p app.mpr -c "describe microflow Bug772.MF_GetOpenTasks" +-- All three groups must appear, and Status must render as the qualified +-- enum value Bug772.TaskStatus.Completed (enrichment reaches later groups too). +-- 3. Feed that describe output back through exec: it re-creates the same flow. +-- + +create module Bug772; +create module role Bug772.User; + +create enumeration Bug772.TaskStatus ( + Open 'Open', + Completed 'Completed' +); + +create persistent entity Bug772.TaskGroup ( + Name: String(100), + EndDate: DateTime +); + +create persistent entity Bug772.Task ( + Title: String(100), + Status: Enumeration(Bug772.TaskStatus), + CompletionDate: DateTime +); + +create association Bug772.Task_TaskGroup + from Bug772.Task to Bug772.TaskGroup + type Reference; + +create microflow Bug772.MF_GetOpenTasks ( + EndDateLimit: DateTime +) +returns list of Bug772.Task as $Tasks +begin + retrieve $Tasks from Bug772.Task + where '[Bug772.Task_TaskGroup/Bug772.TaskGroup[EndDate = $EndDateLimit]][Status != Bug772.TaskStatus.Completed][CompletionDate = empty]'; + return $Tasks; +end; +/ diff --git a/mdl/executor/cmd_microflows_format_action.go b/mdl/executor/cmd_microflows_format_action.go index 88e445230..464e92275 100644 --- a/mdl/executor/cmd_microflows_format_action.go +++ b/mdl/executor/cmd_microflows_format_action.go @@ -429,24 +429,18 @@ func formatAction( // (e.g. Status = 'Open' → Status = Module.OrderStatus.Open) when // the entity is known and we are connected to a project. constraint = enrichXPathConstraintForDescribe(ctx, entityName, constraint) - // XPath may contain multiple predicates like [a][b] or [a]\n[b]. - // Split them and join with MDL 'and' so the parser sees - // separate xpathConstraint nodes. - if strings.HasPrefix(constraint, "[") && strings.HasSuffix(constraint, "]") { - // Split on "][" boundary (possibly separated by \n literals), - // then re-wrap each predicate. - inner := constraint[1 : len(constraint)-1] - // Normalise real newlines between predicates: ]\n[ → ][ - inner = strings.ReplaceAll(inner, "]\n[", "][") - parts := strings.Split(inner, "][") - if len(parts) > 1 { - var wrapped []string - for _, p := range parts { - wrapped = append(wrapped, "["+strings.TrimSpace(p)+"]") - } - constraint = strings.Join(wrapped, "\n ") + // XPath may hold several sibling predicate groups — [a][b], or + // separated by newlines. Emit each on its own line so the parser + // sees separate xpathConstraint nodes. The splitter is nesting- and + // quote-aware: the previous "][" split mangled a group containing a + // nested bracket or a literal with a ']' in it (#772). + if groups := visitor.SplitXPathPredicateGroups(constraint); len(groups) > 0 { + if len(groups) > 1 { + constraint = strings.Join(groups, "\n ") } else { - constraint = parts[0] + // A lone group renders without its outer brackets, matching + // the `where ` form the MDL grammar expects. + constraint = strings.TrimSuffix(strings.TrimPrefix(groups[0], "["), "]") } } stmt += fmt.Sprintf("\n where %s", constraint) @@ -1879,10 +1873,38 @@ func enrichXPathConstraintForDescribe(ctx *ExecContext, entityQN, constraint str if len(enumAttrs) == 0 { return constraint } - expr, ok := visitor.ParseXPathConstraint(constraint) - if !ok || expr == nil { + return enrichXPathGroups(constraint, enumAttrs) +} + +// enrichXPathGroups applies enum enrichment to every top-level predicate group of a +// stored constraint. +// +// Mendix stores sibling groups concatenated (`[a][b][c]`), but the grammar rule +// matches one group. Enriching the whole string at once parsed only the first and +// re-rendered just that, silently discarding the rest — describe then showed a +// materially less restrictive query than the project actually contains +// (mendixlabs/mxcli#772). ParseXPathConstraint now refuses that partial parse, which +// stops the data loss on its own; splitting first is what keeps enrichment working +// for every group rather than only the first. +func enrichXPathGroups(constraint string, enumAttrs map[string]string) string { + groups := visitor.SplitXPathPredicateGroups(constraint) + if len(groups) == 0 { return constraint } - enriched := enrichXPathExprWithEnums(expr, enumAttrs) - return "[" + xpathExprToMDLString(enriched) + "]" + out := make([]string, 0, len(groups)) + for _, g := range groups { + out = append(out, enrichXPathGroup(g, enumAttrs)) + } + return strings.Join(out, "") +} + +// enrichXPathGroup enriches one bracket group, returning it unchanged when it does +// not parse — a group mxcli cannot read is passed through verbatim rather than +// dropped or guessed at. +func enrichXPathGroup(group string, enumAttrs map[string]string) string { + expr, ok := visitor.ParseXPathConstraint(group) + if !ok || expr == nil { + return group + } + return "[" + xpathExprToMDLString(enrichXPathExprWithEnums(expr, enumAttrs)) + "]" } diff --git a/mdl/executor/xpath_enrich_772_test.go b/mdl/executor/xpath_enrich_772_test.go new file mode 100644 index 000000000..ed6ae711e --- /dev/null +++ b/mdl/executor/xpath_enrich_772_test.go @@ -0,0 +1,65 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "strings" + "testing" +) + +// TestEnrichXPathGroups_KeepsEveryGroup is the regression test for +// mendixlabs/mxcli#772. The stored constraint holds three sibling predicate groups; +// enriching the whole string at once parsed only the first, re-rendered just that, +// and dropped the other two — so `describe` showed a materially less restrictive +// query than the project contains, with no warning. +func TestEnrichXPathGroups_KeepsEveryGroup(t *testing.T) { + enumAttrs := map[string]string{"Status": "Reminders.TaskStatus"} + + // Exactly the shape from the issue: a group with a nested bracket, then two + // flat ones, newline-separated. + const stored = "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]\n" + + "[Status != 'Completed']\n[CompletionDate = empty]" + + got := enrichXPathGroups(stored, enumAttrs) + + for _, want := range []string{ + "Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]", + "CompletionDate = empty", + } { + if !strings.Contains(got, want) { + t.Errorf("enrichXPathGroups dropped %q\ngot: %s", want, got) + } + } + // The enum comparison must be enriched to a qualified value — in the *second* + // group, which is the one the old code never reached. + if !strings.Contains(got, "Status != Reminders.TaskStatus.Completed") { + t.Errorf("enum enrichment not applied to a later group\ngot: %s", got) + } + if strings.Contains(got, "'Completed'") { + t.Errorf("enum literal left unenriched\ngot: %s", got) + } +} + +// TestEnrichXPathGroups_PassesThroughUnparseable: a constraint mxcli cannot split +// into groups is returned untouched rather than mangled or dropped. +func TestEnrichXPathGroups_PassesThroughUnparseable(t *testing.T) { + enumAttrs := map[string]string{"Status": "Mod.S"} + for _, in := range []string{ + "Status = 'Open'", // no brackets + "[Status = 'Open'", // unbalanced + "[a = 1] and [b = 2]", // content between groups + } { + if got := enrichXPathGroups(in, enumAttrs); got != in { + t.Errorf("enrichXPathGroups(%q) = %q, want it returned verbatim", in, got) + } + } +} + +// TestEnrichXPathGroups_SingleGroupStillEnriched guards the fix from regressing the +// original single-group behaviour. +func TestEnrichXPathGroups_SingleGroupStillEnriched(t *testing.T) { + got := enrichXPathGroups("[Status = 'Open']", map[string]string{"Status": "Mod.S"}) + if got != "[Status = Mod.S.Open]" { + t.Errorf("enrichXPathGroups = %q, want %q", got, "[Status = Mod.S.Open]") + } +} diff --git a/mdl/visitor/visitor_xpath_public.go b/mdl/visitor/visitor_xpath_public.go index 270b466eb..7a1d0d043 100644 --- a/mdl/visitor/visitor_xpath_public.go +++ b/mdl/visitor/visitor_xpath_public.go @@ -12,6 +12,14 @@ import ( // [ ] brackets stored by Mendix in the XPathConstraint BSON field — and returns the // AST expression. Returns (nil, false) if the input cannot be parsed (e.g. empty, // malformed, or not starting with '['). +// +// The rule matches a SINGLE bracket group. Mendix stores sibling groups +// concatenated — `[a][b][c]` — and with the error listeners removed ANTLR happily +// parsed the first and left the rest on the stream, returning true. Callers read +// that as "fully parsed" and re-rendered only what came back, silently dropping +// every later group (mendixlabs/mxcli#772). A partial parse is therefore reported +// as a failure so callers fall back to the untouched string; use +// SplitXPathPredicateGroups to handle each group in turn. func ParseXPathConstraint(input string) (ast.Expression, bool) { if input == "" { return nil, false @@ -32,5 +40,9 @@ func ParseXPathConstraint(input string) (ast.Expression, bool) { if xpathExpr == nil { return nil, false } + // Anything left on the stream means the rule consumed only a prefix. + if stream.LA(1) != antlr.TokenEOF { + return nil, false + } return buildXPathExpr(xpathExpr), true } diff --git a/mdl/visitor/xpath_groups.go b/mdl/visitor/xpath_groups.go new file mode 100644 index 000000000..8eb8ca539 --- /dev/null +++ b/mdl/visitor/xpath_groups.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import "strings" + +// SplitXPathPredicateGroups splits a stored XPathConstraint into its top-level +// predicate groups, each returned with its enclosing brackets. Mendix concatenates +// sibling groups — `[a][b][c]`, often separated by newlines — and the whole string +// is what the BSON field holds. +// +// Splitting on "][" is not enough: a group may nest brackets of its own +// (`[Mod.Assoc/Mod.Entity[EndDate = $Limit]]`), and a string literal may contain a +// bracket (`[Name = 'a]b']`). Both appear in real projects, and mishandling either +// silently changes the meaning of a query (mendixlabs/mxcli#772). This tracks +// nesting depth and quoting instead. +// +// Returns nil when the input is not a well-formed sequence of bracket groups — +// unbalanced, empty, or with content outside the brackets — so callers can fall +// back to using the string verbatim rather than emit something they invented. +func SplitXPathPredicateGroups(constraint string) []string { + s := strings.TrimSpace(constraint) + if s == "" || !strings.HasPrefix(s, "[") { + return nil + } + + var groups []string + var depth int + var start int + var inQuote bool + + for i := 0; i < len(s); i++ { + c := s[i] + if inQuote { + // Mendix escapes a quote inside a literal by doubling it. + if c == '\'' { + if i+1 < len(s) && s[i+1] == '\'' { + i++ + continue + } + inQuote = false + } + continue + } + switch c { + case '\'': + inQuote = true + case '[': + if depth == 0 { + // Anything between groups must be whitespace only. + if strings.TrimSpace(s[start:i]) != "" { + return nil + } + start = i + } + depth++ + case ']': + depth-- + if depth < 0 { + return nil + } + if depth == 0 { + groups = append(groups, s[start:i+1]) + start = i + 1 + } + } + } + + if depth != 0 || inQuote { + return nil + } + if strings.TrimSpace(s[start:]) != "" { + return nil + } + if len(groups) == 0 { + return nil + } + return groups +} diff --git a/mdl/visitor/xpath_groups_test.go b/mdl/visitor/xpath_groups_test.go new file mode 100644 index 000000000..b7584572c --- /dev/null +++ b/mdl/visitor/xpath_groups_test.go @@ -0,0 +1,79 @@ +// SPDX-License-Identifier: Apache-2.0 + +package visitor + +import ( + "reflect" + "testing" +) + +func TestSplitXPathPredicateGroups(t *testing.T) { + tests := []struct { + name string + in string + want []string + }{ + { + // The constraint from mendixlabs/mxcli#772, verbatim: a group with a + // nested bracket followed by two flat ones, newline-separated. + name: "issue 772 — nested group plus siblings", + in: "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]\n" + + "[Status != 'Completed']\n[CompletionDate = empty]", + want: []string{ + "[Reminders.Task_TaskGroup/Reminders.TaskGroup[EndDate = $EndDateLimit]]", + "[Status != 'Completed']", + "[CompletionDate = empty]", + }, + }, + { + name: "single group", + in: "[Status = 'Open']", + want: []string{"[Status = 'Open']"}, + }, + { + name: "adjacent groups, no separator", + in: "[a = 1][b = 2]", + want: []string{"[a = 1]", "[b = 2]"}, + }, + { + // A "][" split would cut this literal in half. + name: "bracket inside a string literal", + in: "[Name = 'a][b'][Status = 'Open']", + want: []string{"[Name = 'a][b']", "[Status = 'Open']"}, + }, + { + name: "doubled quote escape inside a literal", + in: "[Name = 'it''s]here'][Age = 3]", + want: []string{"[Name = 'it''s]here']", "[Age = 3]"}, + }, + {name: "empty", in: "", want: nil}, + {name: "not bracketed", in: "Status = 'Open'", want: nil}, + {name: "unbalanced open", in: "[a = 1", want: nil}, + {name: "unbalanced close", in: "[a = 1]]", want: nil}, + {name: "content between groups", in: "[a = 1] and [b = 2]", want: nil}, + {name: "unterminated literal", in: "[Name = 'x]", want: nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := SplitXPathPredicateGroups(tc.in) + if !reflect.DeepEqual(got, tc.want) { + t.Errorf("SplitXPathPredicateGroups(%q)\n got %#v\nwant %#v", tc.in, got, tc.want) + } + }) + } +} + +// TestParseXPathConstraint_RejectsPartialParse is the core of #772: the rule matches +// one bracket group, and with the error listeners removed ANTLR parsed the first and +// left the rest on the stream while still reporting success. Callers treated that as +// a full parse and re-rendered only what came back. +func TestParseXPathConstraint_RejectsPartialParse(t *testing.T) { + multi := "[Status != 'Completed'][CompletionDate = empty]" + if _, ok := ParseXPathConstraint(multi); ok { + t.Error("ParseXPathConstraint reported success on a multi-group constraint it only partly consumed") + } + // A single group must still parse. + if _, ok := ParseXPathConstraint("[Status != 'Completed']"); !ok { + t.Error("ParseXPathConstraint rejected a single well-formed group") + } +} From bc8fe241ce31d069cf6a903032c2a327c34170c4 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 04:40:12 +0000 Subject: [PATCH 2/6] fix: security writes stripped inherited members from access rules (#758, #765) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mendix models inheritance across multiple tables: a child adds attributes to the parent's, and all of the parent's are members of the child. An access rule must therefore carry a MemberAccess entry for every member — own AND inherited — or Mendix reports CE0066 "Entity access is out of date". Both the GRANT builder and ReconcileMemberAccesses enumerated only entity.Attributes. Two consequences, and the second explains why the first could not be worked around: * GRANT naming an inherited member produced no entry at all, while reporting success. * Reconciliation runs immediately after every GRANT, and on any write touching the module. An inherited reference is qualified against the entity that DECLARES it, so it never matched the child's own attribute list and was deleted as stale — removing, in the same command, what the grant had just written. That is why REVOKE + GRANT never repaired a damaged rule. The damage was masked: mx check reports CE0066 and stops, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked, so CLI-only workflows shipped it undetected. Two facts were established against mx check rather than inferred: 1. An inherited member's reference must be qualified against its declaring entity. Sec758.Base.SharedField validates clean; the child-qualified Sec758.Item.SharedField is CE1613 "The selected attribute no longer exists". mxcli wrote the child form. This is the same rule the change-object writer needs (#451). 2. System.User's members are the exception. Entities specialising it are user entities whose platform members Mendix manages: listing them turns a clean rule into CE0066 — confirmed on Mendix's own Administration.Account and on a fresh specialisation — while omitting System.FileDocument's six members is CE0066 until all are present. Fixed: * EntityMembers walks the generalization chain, qualifying each member against its declaring entity and excluding System.User's platform members. The GRANT builder uses it, and now rejects a named member that matched nothing instead of dropping it in silence. * Reconciliation strips only a reference qualified to the entity itself. An ancestor may live in another module or in System, neither of which is loaded at that layer, so an inherited reference cannot be validated there — it is preserved rather than deleted. Applied to both engines. Verified end-to-end on a real 11.12.2 project carrying all three specialisation shapes at once — same-module ancestor, System.FileDocument, and System.User — mx check reports 0 errors, and describe round-trips both members of the mixed entity. All three guards mutation-checked. Refs mendixlabs/mxcli#758, mendixlabs/mxcli#765 --- .claude/skills/fix-issue.md | 1 + .../bug-tests/758-inherited-member-access.mdl | 70 ++++++++++ mdl/backend/modelsdk/attr_ref_owner_test.go | 35 +++++ .../modelsdk/domainmodel_security_write.go | 34 ++++- mdl/executor/cmd_security_write.go | 36 ++++- mdl/executor/entity_hierarchy.go | 130 +++++++++++++++++ mdl/executor/entity_hierarchy_test.go | 131 ++++++++++++++++++ modelsdk/mpr/security_patch.go | 34 ++++- sdk/mpr/writer_security.go | 24 +++- 9 files changed, 483 insertions(+), 12 deletions(-) create mode 100644 mdl-examples/bug-tests/758-inherited-member-access.mdl create mode 100644 mdl/backend/modelsdk/attr_ref_owner_test.go create mode 100644 mdl/executor/entity_hierarchy.go create mode 100644 mdl/executor/entity_hierarchy_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index f4b24d120..90a215383 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -316,6 +316,7 @@ cases for these three BSON types — they fell to `default: return nil`. | `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases | | `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests | | `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | +| `UPDATE SECURITY`, `CREATE ASSOCIATION` or any `GRANT` silently strips **inherited** members from a specialized entity's access rules; `GRANT` naming an inherited member reports success and persists nothing, so REVOKE+GRANT cannot repair it. `mx check` shows only CE0066, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked | Mendix inheritance is multi-table: all of a parent's attributes are members of the child, so an access rule needs a MemberAccess entry for every member, own **and** inherited, each qualified against the entity that **declares** it. Both the GRANT builder and `ReconcileMemberAccesses` enumerated only `entity.Attributes`, so an inherited reference matched nothing and was deleted as stale — and reconciliation runs **immediately after every GRANT**, deleting what the grant had just written correctly | `mdl/executor/entity_hierarchy.go` (`EntityMembers`), `mdl/executor/cmd_security_write.go` (`execGrantEntityAccess`), `mdl/backend/modelsdk/domainmodel_security_write.go` (`ReconcileMemberAccesses`, `attrRefBelongsTo`), `sdk/mpr/writer_security.go` (legacy engine) | Walk the generalization chain and qualify each member against its declaring entity; in the reconciler, only strip a reference qualified to **this** entity — an ancestor may live in another module or System, neither loaded there, so preserve what cannot be validated. **Two facts must be established against `mx check`, never inferred**: (a) the child-qualified form is CE1613 "attribute no longer exists" while the declaring-entity form validates clean; (b) `System.User`'s members are the exception — entities specialising it are *user entities* whose platform members Mendix manages, and listing them turns a clean rule into CE0066, while omitting `System.FileDocument`'s six members is CE0066 until all are present. **Generalisable**: when a post-write reconcile pass validates against a narrower model than the writer used, it will quietly undo correct writes — check what runs *after* a write before concluding the writer is at fault. Repro `mdl-examples/bug-tests/758-inherited-member-access.mdl`. Issues #758, #765 (umbrella; #451 is the same declaring-entity rule in the change-object writer) | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/mdl-examples/bug-tests/758-inherited-member-access.mdl b/mdl-examples/bug-tests/758-inherited-member-access.mdl new file mode 100644 index 000000000..b958b02f3 --- /dev/null +++ b/mdl-examples/bug-tests/758-inherited-member-access.mdl @@ -0,0 +1,70 @@ +-- Bug #758 / #765: security reconciliation stripped inherited members, and GRANT +-- could not put them back +-- +-- Mendix models inheritance across multiple tables: a child adds attributes to the +-- parent's, and ALL the parent's attributes are available on the child. An access +-- rule must therefore carry a MemberAccess entry for every member — own AND +-- inherited — or Mendix reports CE0066 "Entity access is out of date". +-- +-- Two facts established against `mx check`, not inferred: +-- +-- 1. An inherited member's reference is qualified against the entity that +-- DECLARES it. `Sec758.Base.SharedField` validates clean; the child-qualified +-- `Sec758.Item.SharedField` is CE1613 "The selected attribute no longer +-- exists". mxcli wrote the child form. +-- +-- 2. Members inherited from System.User are the exception: entities specialising +-- it are user entities whose platform members Mendix manages. Listing them +-- turns a clean rule into CE0066 — confirmed on Mendix's own +-- Administration.Account and on a fresh specialisation. Every other ancestor's +-- members are required: omitting the six System.FileDocument members from a +-- specialising entity's rule is CE0066 until all are present. +-- +-- What went wrong. Both the GRANT path and reconciliation enumerated only +-- entity.Attributes: +-- +-- * GRANT naming an inherited member produced no entry, and reported success. +-- * ReconcileMemberAccesses — which runs immediately after every GRANT, and on +-- any write touching the module — saw the inherited entry's ancestor +-- qualification, failed to match it against the child's own attributes, and +-- deleted it as stale. So the grant wrote the right thing and the reconcile in +-- the same command removed it. That is why REVOKE + GRANT never repaired a +-- damaged rule. +-- +-- The damage was masked: `mx check` reports CE0066 and stops, hiding the CE2729 +-- "No read access to attribute" cascade underneath until Studio Pro's +-- "Update security" is clicked. +-- +-- Verify: +-- 1. mxcli exec 758-inherited-member-access.mdl -p app.mpr +-- 2. mxcli -p app.mpr -c "describe entity Sec758.Item" +-- -> grant ... (read (OwnField, SharedField)); both members present +-- 3. mxcli docker check -p app.mpr -> 0 errors (no CE0066) +-- 4. Repeat step 3 after any further ALTER on the module: the inherited entry +-- must still be there. + +create module Sec758; +create module role Sec758.Editor; + +create persistent entity Sec758.Base ( + SharedField: String(100) +); + +-- Same-module ancestor: mixed own + inherited members. +create persistent entity Sec758.Item extends Sec758.Base ( + OwnField: String(100) +); + +-- System ancestor whose members ARE required in the child's rule. +create persistent entity Sec758.Attachment extends System.FileDocument ( + "Caption": String(200) +); + +-- User entity: System.User's platform members must NOT appear in the rule. +create persistent entity Sec758.Employee extends System.User ( + EmployeeNo: String(20) +); + +grant Sec758.Editor on Sec758.Item (read (SharedField, OwnField)); +grant Sec758.Editor on Sec758.Attachment (read ("Caption")); +grant Sec758.Editor on Sec758.Employee (read (EmployeeNo)); diff --git a/mdl/backend/modelsdk/attr_ref_owner_test.go b/mdl/backend/modelsdk/attr_ref_owner_test.go new file mode 100644 index 000000000..a28b4b4de --- /dev/null +++ b/mdl/backend/modelsdk/attr_ref_owner_test.go @@ -0,0 +1,35 @@ +// SPDX-License-Identifier: Apache-2.0 + +package modelsdkbackend + +import "testing" + +// TestAttrRefBelongsTo pins the distinction reconciliation depends on: only a +// reference qualified against THIS entity names one of its own attributes and can +// be validated from this domain model. An inherited reference carries an +// ancestor's name — possibly from another module or System, neither loaded here — +// and deleting it as "stale" is mendixlabs/mxcli#758. +func TestAttrRefBelongsTo(t *testing.T) { + tests := []struct { + ref string + module string + entity string + want bool + }{ + {"Sec.Item.OwnField", "Sec", "Item", true}, + {"sec.item.OwnField", "Sec", "Item", true}, // Mendix names are case-insensitive + {"Sec.Base.SharedField", "Sec", "Item", false}, + {"System.FileDocument.Name", "Sec", "Attachment", false}, + {"Other.Base.Field", "Sec", "Item", false}, + {"NoDots", "Sec", "Item", false}, + {"", "Sec", "Item", false}, + // A same-named entity in another module is NOT this entity. + {"Other.Item.OwnField", "Sec", "Item", false}, + } + for _, tc := range tests { + if got := attrRefBelongsTo(tc.ref, tc.module, tc.entity); got != tc.want { + t.Errorf("attrRefBelongsTo(%q, %q, %q) = %v, want %v", + tc.ref, tc.module, tc.entity, got, tc.want) + } + } +} diff --git a/mdl/backend/modelsdk/domainmodel_security_write.go b/mdl/backend/modelsdk/domainmodel_security_write.go index 9102579a2..227dca293 100644 --- a/mdl/backend/modelsdk/domainmodel_security_write.go +++ b/mdl/backend/modelsdk/domainmodel_security_write.go @@ -5,6 +5,7 @@ package modelsdkbackend import ( "fmt" "sort" + "strings" "github.com/mendixlabs/mxcli/mdl/backend" "github.com/mendixlabs/mxcli/mdl/types" @@ -412,7 +413,8 @@ func (b *Backend) ReconcileMemberAccesses(unitID model.ID, moduleName string) (i } switch attrRef, assocRef := ma.AttributeQualifiedName(), ma.AssociationQualifiedName(); { case attrRef != "": - if attrSet[attrRef] { + switch { + case attrSet[attrRef]: covAttr[attrRef] = true if calcSet[attrRef] { if r := ma.AccessRights(); r == "ReadWrite" || r == "WriteOnly" { @@ -420,7 +422,21 @@ func (b *Backend) ReconcileMemberAccesses(unitID model.ID, moduleName string) (i changed = true } } - } else { + case !attrRefBelongsTo(attrRef, moduleName, entityName): + // An attribute reference is qualified against the entity that + // DECLARES it, so an inherited member carries an ancestor's name + // rather than this entity's. attrSet holds only this entity's own + // attributes, so every inherited entry looked stale and was + // deleted — silently, on any write touching the module, and + // immediately after the GRANT that had just written it correctly + // (mendixlabs/mxcli#758). The ancestor may live in another module + // or in System, neither loaded here, so an inherited reference + // cannot be validated at this layer at all. Preserve what cannot + // be checked instead of dropping it. + covAttr[attrRef] = true + default: + // Genuinely stale: the reference claims to be this entity's own + // attribute and the entity no longer has it. rule.RemoveMemberAccesses(i) changed = true } @@ -489,3 +505,17 @@ func newMemberAccess(rights, qualifiedName string, isAttr bool) *genDm.MemberAcc assignID(ma) return ma } + +// attrRefBelongsTo reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") names one of the given entity's OWN attributes, +// rather than one inherited from an ancestor. +// +// Only an own reference can be validated from a single domain model: an ancestor +// may live in another module or in System, neither of which is loaded here. +func attrRefBelongsTo(attrRef, moduleName, entityName string) bool { + idx := strings.LastIndex(attrRef, ".") + if idx < 0 { + return false + } + return strings.EqualFold(attrRef[:idx], moduleName+"."+entityName) +} diff --git a/mdl/executor/cmd_security_write.go b/mdl/executor/cmd_security_write.go index beb415345..ffdbb65f8 100644 --- a/mdl/executor/cmd_security_write.go +++ b/mdl/executor/cmd_security_write.go @@ -377,21 +377,30 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error readMemberSet[m] = true } - // Create entries for all entity attributes - for _, attr := range entity.Attributes { + // Create entries for every attribute of the entity's access surface — its own + // AND those inherited through the generalization chain. Enumerating only + // entity.Attributes meant a GRANT naming an inherited member produced no entry + // at all while still reporting success, and left the rule incomplete so Mendix + // reported CE0066 (mendixlabs/mxcli#758). Each reference is qualified against + // the entity that DECLARES the member; qualifying an inherited one against this + // entity is CE1613 "The selected attribute no longer exists". + entityQN := module.Name + "." + s.Entity.Name + members := EntityMembers(ctx, entityQN) + grantedMembers := map[string]bool{} + for _, mem := range members { rights := defaultMemberAccess - if writeMemberSet[attr.Name] { + if writeMemberSet[mem.Name] { rights = "ReadWrite" - } else if readMemberSet[attr.Name] { + } else if readMemberSet[mem.Name] { rights = "ReadOnly" } // Calculated attributes cannot have write rights (CE6592) - isCalculated := attr.Value != nil && attr.Value.Type == "CalculatedValue" - if isCalculated && (rights == "ReadWrite" || rights == "WriteOnly") { + if mem.IsCalculated && (rights == "ReadWrite" || rights == "WriteOnly") { rights = "ReadOnly" } + grantedMembers[mem.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ - AttributeRef: module.Name + "." + s.Entity.Name + "." + attr.Name, + AttributeRef: mem.Ref, AccessRights: rights, }) } @@ -407,6 +416,7 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } else if readMemberSet[assoc.Name] { rights = "ReadOnly" } + grantedMembers[assoc.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ AssociationRef: module.Name + "." + assoc.Name, AccessRights: rights, @@ -421,6 +431,7 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } else if readMemberSet[ca.Name] { rights = "ReadOnly" } + grantedMembers[ca.Name] = true memberAccesses = append(memberAccesses, types.EntityMemberAccess{ AssociationRef: module.Name + "." + ca.Name, AccessRights: rights, @@ -428,6 +439,17 @@ func execGrantEntityAccess(ctx *ExecContext, s *ast.GrantEntityAccessStmt) error } } + // A member named in the GRANT that matched nothing used to be dropped in + // silence — the command reported success and the access simply was not there, + // which is why REVOKE + GRANT could not repair a damaged rule (#758). Name it + // instead. Inherited members now resolve, so anything still unmatched is a typo + // or a member of another entity. + if unknown := unmatchedGrantMembers(readMembers, writeMembers, grantedMembers); len(unknown) > 0 { + return mdlerrors.NewValidationf( + "entity %s has no member(s) %s; grant only names members of the entity or of an entity it inherits from", + entityQN, strings.Join(unknown, ", ")) + } + // Add MemberAccess entries for system associations (owner, changedBy). // When an entity has HasOwner/HasChangedBy, Mendix implicitly adds // System.owner/System.changedBy associations that require MemberAccess. diff --git a/mdl/executor/entity_hierarchy.go b/mdl/executor/entity_hierarchy.go new file mode 100644 index 000000000..45f60dd63 --- /dev/null +++ b/mdl/executor/entity_hierarchy.go @@ -0,0 +1,130 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "sort" + "strings" + + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// userEntityBase is the Mendix entity whose specializations are "user entities". +// Its members are managed by the platform (login, blocking, password), so they do +// NOT belong in a specializing entity's access rule — see EntityMembers. +const userEntityBase = "System.User" + +// EntityMember is one member of an entity's access surface: an attribute or an +// association, together with the qualified reference Mendix stores for it. +type EntityMember struct { + Name string // bare member name, as written in GRANT + // Ref is the reference stored in MemberAccess, qualified against the entity + // that DECLARES the member — which is an ancestor for an inherited one. + Ref string + Inherited bool + IsCalculated bool +} + +// EntityMembers returns every member of an entity's access surface: its own +// attributes plus those inherited through the generalization chain, each carrying +// the reference Mendix expects in a MemberAccess entry. +// +// Two rules here are load-bearing, both established against `mx check` rather than +// inferred (mendixlabs/mxcli#758, #765): +// +// 1. An inherited member's reference is qualified against the entity that +// DECLARES it, not the entity carrying the rule. Writing the child's name +// produces CE1613 "The selected attribute ... no longer exists"; writing the +// declaring entity's name validates clean. This is the same rule the +// change-object writer needs (#451). +// +// 2. Members inherited from System.User are excluded. Mendix manages the platform +// members of a user entity, and listing them turns a clean rule into CE0066 — +// verified both on Mendix's own Administration.Account and on a fresh +// specialization. Every other ancestor's members are REQUIRED: omitting the +// six System.FileDocument members from a specializing entity's rule is CE0066 +// until they are all present. +// +// Ancestors that cannot be resolved (module not in the project) stop the walk; the +// members found so far are returned rather than nothing, so a partial model still +// produces a usable rule. +func EntityMembers(ctx *ExecContext, entityQN string) []EntityMember { + var out []EntityMember + seen := map[string]bool{} // cycle guard + claimed := map[string]bool{} // a child's member shadows the ancestor's + + for currentQN, depth := entityQN, 0; currentQN != ""; depth++ { + if seen[currentQN] { + break + } + seen[currentQN] = true + + // Stop before collecting System.User's own members: its specializations are + // user entities, whose platform members Mendix owns. + if depth > 0 && strings.EqualFold(currentQN, userEntityBase) { + break + } + + entity, ok := findEntityByQN(ctx, currentQN) + if !ok { + break + } + + for _, attr := range entity.Attributes { + if attr == nil || claimed[attr.Name] { + continue + } + claimed[attr.Name] = true + out = append(out, EntityMember{ + Name: attr.Name, + Ref: currentQN + "." + attr.Name, + Inherited: depth > 0, + IsCalculated: attr.Value != nil && attr.Value.Type == "CalculatedValue", + }) + } + currentQN = entity.GeneralizationRef + } + return out +} + +// findEntityByQN resolves a qualified entity name through the backend. +func findEntityByQN(ctx *ExecContext, entityQN string) (*domainmodel.Entity, bool) { + if ctx == nil || ctx.Backend == nil { + return nil, false + } + parts := strings.SplitN(entityQN, ".", 2) + if len(parts) != 2 { + return nil, false + } + mod, err := ctx.Backend.GetModuleByName(parts[0]) + if err != nil || mod == nil { + return nil, false + } + dm, err := ctx.Backend.GetDomainModel(mod.ID) + if err != nil || dm == nil { + return nil, false + } + entity := dm.FindEntityByName(parts[1]) + if entity == nil { + return nil, false + } + return entity, true +} + +// unmatchedGrantMembers returns the members named in a GRANT that matched no +// attribute or association of the entity, in a stable order. +func unmatchedGrantMembers(readMembers, writeMembers []string, granted map[string]bool) []string { + var unknown []string + seen := map[string]bool{} + for _, list := range [][]string{readMembers, writeMembers} { + for _, name := range list { + if name == "" || granted[name] || seen[name] { + continue + } + seen[name] = true + unknown = append(unknown, name) + } + } + sort.Strings(unknown) + return unknown +} diff --git a/mdl/executor/entity_hierarchy_test.go b/mdl/executor/entity_hierarchy_test.go new file mode 100644 index 000000000..d9b4b01a8 --- /dev/null +++ b/mdl/executor/entity_hierarchy_test.go @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/backend/mock" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/domainmodel" +) + +// hierarchyBackend serves three modules: +// +// App.Item extends App.Base (same-module ancestor) +// App.Doc extends System.FileDocument +// App.Employee extends System.User (a user entity) +func hierarchyBackend() *mock.MockBackend { + ids := map[string]model.ID{"App": "mod-app", "System": "mod-system"} + attr := func(name string) *domainmodel.Attribute { + return &domainmodel.Attribute{Name: name} + } + dms := map[model.ID]*domainmodel.DomainModel{ + ids["App"]: {ContainerID: ids["App"], Entities: []*domainmodel.Entity{ + {Name: "Base", Attributes: []*domainmodel.Attribute{attr("SharedField")}}, + {Name: "Item", GeneralizationRef: "App.Base", Attributes: []*domainmodel.Attribute{attr("OwnField")}}, + {Name: "Doc", GeneralizationRef: "System.FileDocument", Attributes: []*domainmodel.Attribute{attr("Caption")}}, + {Name: "Employee", GeneralizationRef: "System.User", Attributes: []*domainmodel.Attribute{attr("EmployeeNo")}}, + }}, + ids["System"]: {ContainerID: ids["System"], Entities: []*domainmodel.Entity{ + {Name: "FileDocument", Attributes: []*domainmodel.Attribute{attr("Name"), attr("Contents")}}, + {Name: "User", Attributes: []*domainmodel.Attribute{attr("Name"), attr("Password")}}, + }}, + } + return &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + id, ok := ids[name] + if !ok { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: id}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dms[id], nil }, + } +} + +func memberRefs(members []EntityMember) []string { + out := make([]string, 0, len(members)) + for _, m := range members { + out = append(out, m.Ref) + } + return out +} + +// TestEntityMembers_InheritedUseDeclaringEntity is the core of +// mendixlabs/mxcli#758 / #765: enumerating only the entity's own attributes meant a +// GRANT naming an inherited member wrote nothing, and reconciliation deleted any +// inherited entry as stale. The reference must be qualified against the entity that +// DECLARES the member — qualifying it against the child is CE1613. +func TestEntityMembers_InheritedUseDeclaringEntity(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Item")) + want := []string{"App.Item.OwnField", "App.Base.SharedField"} + if len(got) != len(want) { + t.Fatalf("EntityMembers(App.Item) = %v, want %v", got, want) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("member %d = %q, want %q", i, got[i], want[i]) + } + } +} + +// TestEntityMembers_SystemAncestorIncluded: omitting the System.FileDocument +// members from a specializing entity's rule is CE0066 until they are all present, +// so they must be enumerated. +func TestEntityMembers_SystemAncestorIncluded(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Doc")) + for _, want := range []string{"App.Doc.Caption", "System.FileDocument.Name", "System.FileDocument.Contents"} { + found := false + for _, g := range got { + if g == want { + found = true + } + } + if !found { + t.Errorf("EntityMembers(App.Doc) missing %q; got %v", want, got) + } + } +} + +// TestEntityMembers_UserEntityExcludesSystemUser: Mendix manages the platform +// members of a user entity. Listing them turns a clean rule into CE0066 — +// verified against Mendix's own Administration.Account and a fresh specialization. +func TestEntityMembers_UserEntityExcludesSystemUser(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + got := memberRefs(EntityMembers(ctx, "App.Employee")) + if len(got) != 1 || got[0] != "App.Employee.EmployeeNo" { + t.Errorf("EntityMembers(App.Employee) = %v, want only its own member — "+ + "System.User's platform members must not appear", got) + } +} + +// TestEntityMembers_ChildShadowsAncestor: a child redeclaring an ancestor's +// attribute name must contribute its own reference once, not both. +func TestEntityMembers_ChildShadowsAncestor(t *testing.T) { + ctx, _ := newMockCtx(t, withBackend(hierarchyBackend())) + + // App.Doc declares Caption; System.FileDocument declares Name/Contents. Add a + // shadowing case by checking no member name appears twice. + seen := map[string]bool{} + for _, m := range EntityMembers(ctx, "App.Doc") { + if seen[m.Name] { + t.Errorf("member %q enumerated twice", m.Name) + } + seen[m.Name] = true + } +} + +func TestUnmatchedGrantMembers(t *testing.T) { + granted := map[string]bool{"OwnField": true, "SharedField": true} + got := unmatchedGrantMembers([]string{"SharedField", "Nope"}, []string{"OwnField", "Alsobad"}, granted) + if len(got) != 2 || got[0] != "Alsobad" || got[1] != "Nope" { + t.Errorf("unmatchedGrantMembers = %v, want [Alsobad Nope]", got) + } +} diff --git a/modelsdk/mpr/security_patch.go b/modelsdk/mpr/security_patch.go index 28e66bacd..c6059ab87 100644 --- a/modelsdk/mpr/security_patch.go +++ b/modelsdk/mpr/security_patch.go @@ -4,6 +4,7 @@ package mpr import ( "fmt" + "strings" "go.mongodb.org/mongo-driver/v2/bson" ) @@ -286,14 +287,28 @@ func secPatchReconcileMemberAccessesDoc(doc bson.D, moduleName string) (bson.D, if attrRef != "" { parts := secSplitQualifiedRef(attrRef) - if parts != "" && attrNames[parts] { + // An attribute reference is qualified against the entity that + // DECLARES it, so an inherited member carries an ancestor's + // name, not this entity's. Reconciliation used to compare only + // the bare member name against this entity's own attributes, + // so every inherited entry looked stale and was deleted — + // silently, and on any write that touched the module + // (mendixlabs/mxcli#758). The ancestor may live in another + // module or in System, neither of which is present in this + // document, so an inherited reference cannot be validated here + // at all: preserve it rather than delete what cannot be + // checked. + if !secRefBelongsToEntity(attrRef, moduleName, entityName) { + filtered = append(filtered, maDoc) + } else if parts != "" && attrNames[parts] { coveredAttrs[parts] = true if calculatedAttrs[parts] { maDoc = secDowngradeCalculatedAttrRights(maDoc) } filtered = append(filtered, maDoc) } else { - // Stale attribute ref (attribute was deleted or renamed). + // Genuinely stale: the reference claims to be this entity's + // own attribute, and the entity no longer has it. changes = append(changes, ReconcileChange{Entity: entityName, Member: parts, Action: "stripped"}) changed = true } @@ -500,3 +515,18 @@ func secStripInvalidAccessRuleProps(doc bson.D) (bson.D, bool) { } return cleaned, stripped } + +// secRefBelongsToEntity reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") is qualified against the given entity — i.e. names +// one of its OWN attributes rather than one inherited from an ancestor. +// +// Only an own reference can be validated from this document: an ancestor may live +// in another module or in System, neither of which is loaded here. +func secRefBelongsToEntity(attrRef, moduleName, entityName string) bool { + parts := secSplitByDot(attrRef) + if len(parts) < 3 { + return false + } + owner := strings.Join(parts[:len(parts)-1], ".") + return strings.EqualFold(owner, moduleName+"."+entityName) +} diff --git a/sdk/mpr/writer_security.go b/sdk/mpr/writer_security.go index 15b05e148..b40e006f9 100644 --- a/sdk/mpr/writer_security.go +++ b/sdk/mpr/writer_security.go @@ -1472,7 +1472,17 @@ func (w *Writer) ReconcileMemberAccesses(unitID model.ID, moduleName string) (in if attrRef != "" { // Extract attribute name from Module.Entity.AttrName parts := splitQualifiedRef(attrRef) - if parts != "" && attrNames[parts] { + // An inherited member's reference is qualified against the + // entity that DECLARES it, so it does not match this + // entity's own attribute list and used to be deleted as + // stale (mendixlabs/mxcli#758). The ancestor may live in + // another module or in System, neither loaded here, so an + // inherited reference cannot be validated at this layer — + // preserve what cannot be checked. Mirrors the codec engine + // (mdl/backend/modelsdk.attrRefBelongsTo). + if !attrRefBelongsToEntity(attrRef, moduleName, entityName) { + filtered = append(filtered, maDoc) + } else if parts != "" && attrNames[parts] { coveredAttrs[parts] = true // Downgrade write rights on calculated attributes (CE6592) if calculatedAttrs[parts] { @@ -1653,3 +1663,15 @@ func stripInvalidAccessRuleProps(doc bson.D) (bson.D, bool) { // ensure primitive import is used var _ = primitive.Binary{} + +// attrRefBelongsToEntity reports whether a MemberAccess attribute reference +// ("Module.Entity.Attribute") names one of the given entity's OWN attributes, +// rather than one inherited from an ancestor. Only an own reference can be +// validated from a single domain model. +func attrRefBelongsToEntity(attrRef, moduleName, entityName string) bool { + idx := strings.LastIndex(attrRef, ".") + if idx < 0 { + return false + } + return strings.EqualFold(attrRef[:idx], moduleName+"."+entityName) +} From ccbc4761d3a2f2da0ad35a48b3031ae69e6b73e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 05:03:03 +0000 Subject: [PATCH 3/6] docs: document security for inherited members MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing about entity inheritance appeared in any security doc, even though a specialized entity's access rule must cover its inherited members and getting it wrong is CE0066. Added to each surface the story touches: - mxcli syntax security.entity-access — an "Inherited members" block plus examples for a same-module ancestor and System.FileDocument - skills/mendix/manage-security.md — worked example, the None-rights detail, the new unknown-member error, and the System.User exception - skills/mendix/generate-domain-model.md — a pointer from EXTENDS, where a reader meets inheritance first - docs-site security/grant.md — the same as reference prose - MDL_QUICK_REFERENCE.md — the grant-entity-access row Covers what #758/#765 made work: inherited members are named exactly like the entity's own, READ */WRITE * include them, unmatched names are an error rather than a silent skip, and entities extending System.User must not grant their inherited platform members. --- .../skills/mendix/generate-domain-model.md | 9 ++++ .claude/skills/mendix/manage-security.md | 52 +++++++++++++++++++ cmd/mxcli/syntax/features_security.go | 20 ++++++- docs-site/src/reference/security/grant.md | 39 ++++++++++++++ docs/01-project/MDL_QUICK_REFERENCE.md | 2 +- 5 files changed, 119 insertions(+), 3 deletions(-) diff --git a/.claude/skills/mendix/generate-domain-model.md b/.claude/skills/mendix/generate-domain-model.md index 1e62b7acd..633d04961 100644 --- a/.claude/skills/mendix/generate-domain-model.md +++ b/.claude/skills/mendix/generate-domain-model.md @@ -210,6 +210,15 @@ create persistent entity Module.Photo ( **Note:** `mxcli syntax entity` output may show EXTENDS after `)` — this is misleading. Always place EXTENDS before `(`. +**Security follows inheritance.** Mendix inheritance is multi-table: all of the +parent's attributes are members of the child, so a specialized entity's access rule +must cover them. Grant an inherited member exactly like one of the entity's own — +`grant Module.Viewer on Module.Attachment (read (AttachmentDescription, "Name", Size));` +— and `read *` / `write *` cover them too. Skipping them is Mendix CE0066 "Entity +access is out of date". The one exception is entities extending `System.User`, whose +inherited platform members Mendix manages and which must not be granted. See +`manage-security.md`. + #### System Attributes (Auditing) Mendix supports four built-in auditing properties on persistent entities. Declare them as regular attributes using pseudo-types (like `autonumber`): diff --git a/.claude/skills/mendix/manage-security.md b/.claude/skills/mendix/manage-security.md index ac6115c22..425a72e4b 100644 --- a/.claude/skills/mendix/manage-security.md +++ b/.claude/skills/mendix/manage-security.md @@ -167,6 +167,58 @@ revoke MyModule.User on MyModule.Customer (write (Email)); revoke MyModule.User on MyModule.Customer (delete); ``` +#### Inherited members + +Mendix inheritance is multi-table: a child adds attributes to its parent's, and +**all** the parent's members belong to the child. Grant them exactly like the +entity's own — `read *` / `write *` cover them too: + +```sql +create persistent entity Docs.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +create persistent entity Docs.Contract extends Docs.DocumentBase ( + ContractNumber: String(50) +); + +-- DocName is inherited, ContractNumber is Contract's own — name both the same way +grant Docs.Viewer on Docs.Contract (read (DocName, ContractNumber)); + +-- Attachment inherits the file members from System.FileDocument +create persistent entity Docs.Attachment extends System.FileDocument ( + Category: String(50) +); +grant Docs.Viewer on Docs.Attachment (read (Category, "Name", Size)); +``` + +An access rule must carry an entry for **every** member, own and inherited — +mxcli writes the ones you did not grant with rights `None`. Omitting them is +Mendix **CE0066** "Entity access is out of date", which masks the CE2729 +"No read access to attribute" errors underneath until Studio Pro's *Update +security* is clicked. + +A member name that matches nothing is now an error rather than a silent skip: + +``` +Error: entity Docs.Contract has no member(s) DocNam; grant only names members +of the entity or of an entity it inherits from +``` + +**Exception — user entities.** An entity extending `System.User` is a *user +entity*, and Mendix manages its inherited platform members (`Name`, `Password`, +`Blocked`, …). Those must **not** appear in the rule; listing them is CE0066. +Grant only the entity's own members — mxcli leaves the platform ones out +automatically: + +```sql +create persistent entity Docs.Employee extends System.User ( + EmployeeNo: String(20) +); +grant Docs.Viewer on Docs.Employee (read (EmployeeNo)); -- not Name/Blocked +``` + ### User Roles ```sql diff --git a/cmd/mxcli/syntax/features_security.go b/cmd/mxcli/syntax/features_security.go index 0312bcf0d..9f6eba6f5 100644 --- a/cmd/mxcli/syntax/features_security.go +++ b/cmd/mxcli/syntax/features_security.go @@ -33,8 +33,24 @@ func init() { "entity access", "grant", "revoke", "read", "write", "create", "delete", "xpath", "row-level security", }, - Syntax: "GRANT ON . () [WHERE ''];\nREVOKE ON .;\nREVOKE ON . ();\n\nRights: CREATE, DELETE, READ *, READ (,...), WRITE *, WRITE (,...)", - Example: "GRANT Shop.Admin ON Shop.Customer (CREATE, DELETE, READ *, WRITE *);\nGRANT Shop.User ON Shop.Customer (READ *) WHERE '[Active = true()]';", + Syntax: "GRANT ON . () [WHERE ''];\n" + + "REVOKE ON .;\n" + + "REVOKE ON . ();\n\n" + + "Rights: CREATE, DELETE, READ *, READ (,...), WRITE *, WRITE (,...)\n\n" + + "Inherited members:\n" + + " Mendix inheritance is multi-table — a child adds attributes to its\n" + + " parent's, and ALL the parent's members belong to the child. Name them\n" + + " in a GRANT exactly like the entity's own; READ */WRITE * covers them\n" + + " too. A name that matches no member is an error, not a silent skip.\n\n" + + " Exception: entities extending System.User are user entities, whose\n" + + " platform members (Name, Password, Blocked, ...) Mendix manages. Do not\n" + + " grant those; mxcli leaves them out of the rule automatically.", + Example: "GRANT Shop.Admin ON Shop.Customer (CREATE, DELETE, READ *, WRITE *);\n" + + "GRANT Shop.User ON Shop.Customer (READ *) WHERE '[Active = true()]';\n\n" + + "-- Contract extends DocumentBase: DocName is inherited, ContractNumber is own\n" + + "GRANT Docs.Viewer ON Docs.Contract (READ (DocName, ContractNumber));\n\n" + + "-- Attachment extends System.FileDocument: Name and Size are inherited\n" + + "GRANT Docs.Viewer ON Docs.Attachment (READ (Category, \"Name\", Size));", SeeAlso: []string{"security.module-role", "security.microflow-access"}, }) diff --git a/docs-site/src/reference/security/grant.md b/docs-site/src/reference/security/grant.md index 7fe165ad1..1f3e0bb0f 100644 --- a/docs-site/src/reference/security/grant.md +++ b/docs-site/src/reference/security/grant.md @@ -125,6 +125,45 @@ GRANT Shop.Viewer ON Shop.Customer (READ (Phone)); -- Result: READ (Name, Email, Phone) ``` +## Inherited members + +Mendix inheritance is multi-table: a child adds attributes to its parent's, and +**all** the parent's members belong to the child. Name an inherited member in a +GRANT exactly like one of the entity's own; `READ *` and `WRITE *` cover them too. + +```sql +CREATE PERSISTENT ENTITY Docs.DocumentBase (DocName: String(200)); +CREATE PERSISTENT ENTITY Docs.Contract EXTENDS Docs.DocumentBase (ContractNumber: String(50)); + +-- DocName inherited, ContractNumber own — no distinction at the call site +GRANT Docs.Viewer ON Docs.Contract (READ (DocName, ContractNumber)); +``` + +An access rule must carry an entry for **every** member, own and inherited. mxcli +writes the members you did not grant with rights `None`; omitting them entirely is +Mendix **CE0066** *"Entity access is out of date"*, which masks the CE2729 +*"No read access to attribute"* errors beneath it until Studio Pro's +**Update security** is clicked. + +A member name that matches nothing is rejected rather than skipped: + +``` +Error: entity Docs.Contract has no member(s) DocNam; grant only names members +of the entity or of an entity it inherits from +``` + +### User entities are the exception + +An entity extending `System.User` is a *user entity*, and Mendix manages its +inherited platform members (`Name`, `Password`, `Blocked`, …). Those must **not** +appear in the access rule — listing them is CE0066. Grant only the entity's own +members; mxcli excludes the platform ones automatically. + +```sql +CREATE PERSISTENT ENTITY Docs.Employee EXTENDS System.User (EmployeeNo: String(20)); +GRANT Docs.Viewer ON Docs.Employee (READ (EmployeeNo)); +``` + ## See Also [REVOKE](revoke.md), [CREATE MODULE ROLE](create-module-role.md), [CREATE USER ROLE](create-user-role.md) diff --git a/docs/01-project/MDL_QUICK_REFERENCE.md b/docs/01-project/MDL_QUICK_REFERENCE.md index 0804ea41a..cc2925ceb 100644 --- a/docs/01-project/MDL_QUICK_REFERENCE.md +++ b/docs/01-project/MDL_QUICK_REFERENCE.md @@ -366,7 +366,7 @@ Nested folders use `/` separator: `'Parent/Child/Grandchild'`. Missing folders a | Revoke nanoflow access | `revoke execute on nanoflow Mod.NF from Mod.Role, ...;` | | | Grant page access | `grant view on page Mod.Page to Mod.Role, ...;` | | | Revoke page access | `revoke view on page Mod.Page from Mod.Role, ...;` | | -| Grant entity access | `grant Mod.Role on Mod.Entity (create, delete, read *, write *);` | Additive — merges with existing | +| Grant entity access | `grant Mod.Role on Mod.Entity (create, delete, read *, write *);` | Additive — merges with existing. Inherited members are named like the entity's own (`read *` covers them); an unknown name is an error. Entities extending `System.User` are the exception — their platform members must not be granted | | Revoke entity access | `revoke Mod.Role on Mod.Entity;` | Full revoke — removes entire rule | | Revoke entity access (partial) | `revoke Mod.Role on Mod.Entity (read (attr));` | Partial — downgrades specific rights | | Set security level | `alter project security level off\|prototype\|production;` | | From f7dfd6ec465879733dbe3b1867868c0b96b9ebb5 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 05:32:48 +0000 Subject: [PATCH 4/6] fix: mappings skipped attributes inherited from a parent entity (#703) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mendix inheritance is multi-table: a child adds attributes to its parent's, and all the parent's are members of the child. A mapping element bound to one must reference the entity that DECLARES it. The builder prefixed the entity being mapped, unconditionally: attr := def.Attribute if parentEntity != "" && !strings.Contains(attr, ".") { attr = parentEntity + "." + attr // always the CHILD } so every inherited field produced a reference to an attribute that entity does not have. Studio Pro shows the field unmapped — the reported symptom — and mx check reports CE1613 "The selected attribute ... no longer exists". A second, quieter defect sat next to it: resolveAttributeType scanned only the entity's own attributes and fell through to a "String" default, so an inherited Boolean or DateTime element carried the wrong DataType even once the reference was correct. That function also matched entities by name across every domain model, ignoring the module, so a same-named entity elsewhere could win; it now resolves the module by name. Both the import and export builders carried the same two lines, and both are fixed. They route through the generalization walk added for #758, generalised here into ResolveMemberRef (declaring-entity reference) and ResolveMemberType (type from up the chain), each falling back to the previous behaviour when the member cannot be resolved. EntityMembersFor takes the backend directly so the mapping builders, which hold no ExecContext, can use it. This closes the mapping half of the #765 umbrella; the same declaring-entity rule governs entity access rules (#758) and the change-object writer (#451). Verified end-to-end on a real 11.12.2 project with an entity extending another, mapping one own and two inherited attributes in both directions: before: Map703.Contract.DocName StringType -> CE1613 Map703.Contract.Confidential StringType -> CE1613 after: Map703.DocumentBase.DocName StringType Map703.DocumentBase.Confidential BooleanType mx check -> 0 errors Both halves mutation-checked, including a test at the resolveAttributeType call site rather than only on the resolver — reverting the call site alone left the resolver's own test green. Docs: inheritance was unmentioned in every mapping doc, so the syntax topic, the json-structures-and-mappings skill and docs-site create-import-mapping now cover it. Refs mendixlabs/mxcli#703, mendixlabs/mxcli#765 --- .claude/skills/fix-issue.md | 1 + .../mendix/json-structures-and-mappings.md | 35 +++++- cmd/mxcli/syntax/features_integration.go | 10 +- .../integration/create-import-mapping.md | 26 +++++ .../703-mapping-inherited-attributes.mdl | 73 +++++++++++++ mdl/executor/cmd_export_mappings.go | 11 +- mdl/executor/cmd_import_mappings.go | 23 +++- mdl/executor/entity_hierarchy.go | 78 ++++++++++++-- mdl/executor/entity_hierarchy_test.go | 102 ++++++++++++++++++ 9 files changed, 349 insertions(+), 10 deletions(-) create mode 100644 mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index a3a251cbd..5ff838e0d 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -318,6 +318,7 @@ cases for these three BSON types — they fell to `default: return nil`. | `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | | `UPDATE SECURITY`, `CREATE ASSOCIATION` or any `GRANT` silently strips **inherited** members from a specialized entity's access rules; `GRANT` naming an inherited member reports success and persists nothing, so REVOKE+GRANT cannot repair it. `mx check` shows only CE0066, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked | Mendix inheritance is multi-table: all of a parent's attributes are members of the child, so an access rule needs a MemberAccess entry for every member, own **and** inherited, each qualified against the entity that **declares** it. Both the GRANT builder and `ReconcileMemberAccesses` enumerated only `entity.Attributes`, so an inherited reference matched nothing and was deleted as stale — and reconciliation runs **immediately after every GRANT**, deleting what the grant had just written correctly | `mdl/executor/entity_hierarchy.go` (`EntityMembers`), `mdl/executor/cmd_security_write.go` (`execGrantEntityAccess`), `mdl/backend/modelsdk/domainmodel_security_write.go` (`ReconcileMemberAccesses`, `attrRefBelongsTo`), `sdk/mpr/writer_security.go` (legacy engine) | Walk the generalization chain and qualify each member against its declaring entity; in the reconciler, only strip a reference qualified to **this** entity — an ancestor may live in another module or System, neither loaded there, so preserve what cannot be validated. **Two facts must be established against `mx check`, never inferred**: (a) the child-qualified form is CE1613 "attribute no longer exists" while the declaring-entity form validates clean; (b) `System.User`'s members are the exception — entities specialising it are *user entities* whose platform members Mendix manages, and listing them turns a clean rule into CE0066, while omitting `System.FileDocument`'s six members is CE0066 until all are present. **Generalisable**: when a post-write reconcile pass validates against a narrower model than the writer used, it will quietly undo correct writes — check what runs *after* a write before concluding the writer is at fault. Repro `mdl-examples/bug-tests/758-inherited-member-access.mdl`. Issues #758, #765 (umbrella; #451 is the same declaring-entity rule in the change-object writer) | | `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 | +| An import/export mapping over an entity created with `EXTENDS` maps only its **own** attributes; every inherited field shows unmapped in Studio Pro, and `mx check` reports CE1613 "The selected attribute 'Mod.Child.Attr' no longer exists". An inherited Boolean/DateTime element also gets `DataType=String` | The mapping builder prefixed the entity being mapped unconditionally (`attr = parentEntity + "." + attr`), but a member reference is qualified against the entity that **declares** it — the same rule as entity access rules (#758) and the change-object writer (#451). Separately `resolveAttributeType` scanned only the entity's own attributes and fell through to its `"String"` default | `mdl/executor/cmd_import_mappings.go` and `cmd_export_mappings.go` (both carry the same two lines), `mdl/executor/entity_hierarchy.go` (`ResolveMemberRef`, `ResolveMemberType`) | Route both sites through the generalization walk added for #758: `ResolveMemberRef` returns the declaring-entity reference and `ResolveMemberType` finds the type up the chain, each falling back to the old behaviour when the member cannot be resolved. **Watch for the sibling defect**: the old `resolveAttributeType` matched entities **by name across every domain model**, so a same-named entity in another module could win — resolve the module by name instead. **Generalisable**: when one rule has several call sites, a fix at one of them proves nothing about the others; grep for the *pattern* (`range entity.Attributes`, `parentEntity + "."`) rather than the reported symptom. Repro `mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl`; A/B on the same project shows `Map703.Contract.DocName` (CE1613) become `Map703.DocumentBase.DocName`. Issue #703, umbrella #765 | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/.claude/skills/mendix/json-structures-and-mappings.md b/.claude/skills/mendix/json-structures-and-mappings.md index 8d88564bd..d709bf41d 100644 --- a/.claude/skills/mendix/json-structures-and-mappings.md +++ b/.claude/skills/mendix/json-structures-and-mappings.md @@ -10,7 +10,40 @@ A JSON structure defines the schema of a JSON payload. It stores a JSON snippet ### Import Mappings An import mapping converts a JSON string into Mendix entity objects. It maps JSON fields to entity attributes. -### Export Mappings +#### Inherited attributes + +Mendix inheritance is multi-table: all of a parent's attributes are members of the +child, so an entity created with `extends` can map them. Name an inherited +attribute exactly like one of the entity's own — mxcli resolves each to the entity +that **declares** it, which is the reference Studio Pro needs to show the field +mapped. + +```sql +create persistent entity Docs.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +create persistent entity Docs.Contract extends Docs.DocumentBase ( + ContractNumber: String(50) +); + +create import mapping Docs.IMM_Contract + with json structure Docs.JSON_Contract +{ + create Docs.Contract { + ContractNumber = contractNumber, -- own + DocName = docName, -- inherited + Confidential = confidential -- inherited + } +}; +``` + +Qualifying an inherited attribute against the entity being mapped instead of its +declaring entity is Mendix **CE1613** "The selected attribute ... no longer +exists", and the field shows unmapped in Studio Pro. + +## Export Mappings An export mapping converts Mendix entity objects into a JSON string. It maps entity attributes to JSON fields. ### Critical: Import and Export Need Different Domain Models diff --git a/cmd/mxcli/syntax/features_integration.go b/cmd/mxcli/syntax/features_integration.go index 17351ae60..aec81f788 100644 --- a/cmd/mxcli/syntax/features_integration.go +++ b/cmd/mxcli/syntax/features_integration.go @@ -393,7 +393,15 @@ func init() { "show import mappings", "describe import mapping", "with json structure", "find or create", "object handling", }, - Syntax: "SHOW IMPORT MAPPINGS [IN Module];\nDESCRIBE IMPORT MAPPING Module.Name;\nCREATE [OR MODIFY] IMPORT MAPPING Module.Name\n WITH JSON STRUCTURE Module.JsonStruct\n{\n create|find|find or create Module.Entity {\n Attr = jsonField [KEY],\n Assoc/Module.Child = nestedKey { ... }\n }\n};\nDROP IMPORT MAPPING Module.Name;\n\nOR MODIFY: updates mapping in-place, preserves UUID.", + Syntax: "SHOW IMPORT MAPPINGS [IN Module];\nDESCRIBE IMPORT MAPPING Module.Name;\n" + + "CREATE [OR MODIFY] IMPORT MAPPING Module.Name\n WITH JSON STRUCTURE Module.JsonStruct\n{\n" + + " create|find|find or create Module.Entity {\n Attr = jsonField [KEY],\n" + + " Assoc/Module.Child = nestedKey { ... }\n }\n};\nDROP IMPORT MAPPING Module.Name;\n\n" + + "OR MODIFY: updates mapping in-place, preserves UUID.\n\n" + + "Inherited attributes:\n" + + " An entity mapped with EXTENDS can map its inherited attributes too —\n" + + " name them exactly like its own. mxcli resolves each to the entity that\n" + + " declares it, which is what Studio Pro needs to show the field mapped.", Example: "CREATE IMPORT MAPPING Shop.IMM_Order\n WITH JSON STRUCTURE Shop.JSON_Order\n{\n create Shop.Order {\n OrderId = orderId KEY,\n TotalAmount = total\n }\n};\n\n-- Idempotent update\nCREATE OR MODIFY IMPORT MAPPING Shop.IMM_Order\n WITH JSON STRUCTURE Shop.JSON_Order\n{\n find or create Shop.Order {\n OrderId = orderId KEY,\n TotalAmount = total,\n Status = status\n }\n};", SeeAlso: []string{"export-mapping", "json-structure"}, }) diff --git a/docs-site/src/reference/integration/create-import-mapping.md b/docs-site/src/reference/integration/create-import-mapping.md index 9fbf7e2e0..490c1999c 100644 --- a/docs-site/src/reference/integration/create-import-mapping.md +++ b/docs-site/src/reference/integration/create-import-mapping.md @@ -141,6 +141,32 @@ CREATE OR MODIFY IMPORT MAPPING MyModule.IMM_Pet - Arrays in the JSON sample map directly to child entity objects — there is no intermediate container entity (unlike export mappings). - Import and export of the same JSON typically require different entity structures because of the FK direction difference. +## Inherited attributes + +Mendix inheritance is multi-table: all of a parent's attributes are members of the +child, so an entity declared with `EXTENDS` can map them. Name an inherited +attribute exactly like one of the entity's own; mxcli resolves each to the entity +that **declares** it. + +```sql +CREATE PERSISTENT ENTITY Docs.DocumentBase (DocName: String(200), Confidential: Boolean); +CREATE PERSISTENT ENTITY Docs.Contract EXTENDS Docs.DocumentBase (ContractNumber: String(50)); + +CREATE IMPORT MAPPING Docs.IMM_Contract + WITH JSON STRUCTURE Docs.JSON_Contract +{ + create Docs.Contract { + ContractNumber = contractNumber, + DocName = docName, + Confidential = confidential + } +}; +``` + +Referencing an inherited attribute against the entity being mapped rather than its +declaring entity is Mendix **CE1613** *"The selected attribute ... no longer +exists"*, and Studio Pro shows the field unmapped. + ## See Also [CREATE JSON STRUCTURE](create-json-structure.md), [CREATE EXPORT MAPPING](create-export-mapping.md) diff --git a/mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl b/mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl new file mode 100644 index 000000000..87474e738 --- /dev/null +++ b/mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl @@ -0,0 +1,73 @@ +-- Bug #703: import/export mappings silently skipped inherited attributes +-- +-- Mendix inheritance is multi-table: a child adds attributes to its parent's, and +-- all the parent's are members of the child. A mapping element bound to one of +-- them must reference the entity that DECLARES it — `Map703.DocumentBase.DocName`, +-- not `Map703.Contract.DocName`. +-- +-- mxcli prefixed the entity being mapped, unconditionally: +-- +-- attr := def.Attribute +-- if parentEntity != "" && !strings.Contains(attr, ".") { +-- attr = parentEntity + "." + attr // always the CHILD +-- } +-- +-- so every inherited field produced a reference to an attribute that does not +-- exist on that entity. In Studio Pro the field shows as unmapped — the reported +-- symptom — and `mx check` reports CE1613 "The selected attribute ... no longer +-- exists". +-- +-- A second, quieter defect sat alongside it: resolveAttributeType scanned only the +-- entity's own attributes and fell through to a "String" default, so an inherited +-- Boolean or DateTime element got the wrong DataType even once the reference was +-- right. (That function also matched entities by name across every domain model, +-- ignoring the module, so a same-named entity elsewhere could win; it now resolves +-- the module by name.) +-- +-- This is the mapping half of the #765 umbrella. The same declaring-entity rule +-- governs entity access rules (#758) and the change-object writer (#451). +-- +-- Verify: +-- 1. mxcli exec 703-mapping-inherited-attributes.mdl -p app.mpr +-- 2. mxcli docker check -p app.mpr -> 0 errors (no CE1613) +-- 3. Dump the mapping unit: the inherited elements must read +-- Attribute=Map703.DocumentBase.DocName DataType=DataTypes$StringType +-- Attribute=Map703.DocumentBase.Confidential DataType=DataTypes$BooleanType +-- Before the fix both read Map703.Contract.* and both were StringType. +-- 4. Open the mapping in Studio Pro: all three fields are mapped. + +create module Map703; + +create json structure Map703.JSON_Contract +snippet '{"docName": "NDA", "confidential": true, "contractNumber": "C-1"}'; + +-- Parent supplies DocName (String) and Confidential (Boolean). +create persistent entity Map703.DocumentBase ( + DocName: String(200), + Confidential: Boolean +); + +-- Child adds its own; inherits the two above. +create persistent entity Map703.Contract extends Map703.DocumentBase ( + ContractNumber: String(50) +); + +create import mapping Map703.IMM_Contract + with json structure Map703.JSON_Contract +{ + create Map703.Contract { + ContractNumber = contractNumber, + DocName = docName, + Confidential = confidential + } +}; + +create export mapping Map703.EMM_Contract + with json structure Map703.JSON_Contract +{ + Map703.Contract { + contractNumber = ContractNumber, + docName = DocName, + confidential = Confidential + } +}; diff --git a/mdl/executor/cmd_export_mappings.go b/mdl/executor/cmd_export_mappings.go index 1242a1925..5cbb2aaf4 100644 --- a/mdl/executor/cmd_export_mappings.go +++ b/mdl/executor/cmd_export_mappings.go @@ -365,9 +365,18 @@ func buildExportMappingElementModel(moduleName string, def *ast.ExportMappingEle elem.Kind = "Value" elem.TypeName = "ExportMappings$ValueMappingElement" elem.DataType = resolveAttributeType(parentEntity, def.Attribute, b) + // A member reference is qualified against the entity that DECLARES it, so + // an inherited attribute carries an ancestor's name. Prefixing the entity + // being mapped produced CE1613 "The selected attribute no longer exists" + // and left the field unmapped in Studio Pro (mendixlabs/mxcli#703) — the + // same rule as entity access rules (#758), both under the #765 umbrella. attr := def.Attribute if parentEntity != "" && !strings.Contains(attr, ".") { - attr = parentEntity + "." + attr + if ref, ok := ResolveMemberRef(b, parentEntity, attr); ok { + attr = ref + } else { + attr = parentEntity + "." + attr + } } elem.Attribute = attr // JsonPath already set from JSON structure clone above diff --git a/mdl/executor/cmd_import_mappings.go b/mdl/executor/cmd_import_mappings.go index f31b7ca68..3499a1f33 100644 --- a/mdl/executor/cmd_import_mappings.go +++ b/mdl/executor/cmd_import_mappings.go @@ -341,9 +341,18 @@ func buildImportMappingElementModel(moduleName string, def *ast.ImportMappingEle elem.TypeName = "ImportMappings$ValueMappingElement" elem.DataType = resolveAttributeType(parentEntity, def.Attribute, b) elem.IsKey = def.IsKey + // A member reference is qualified against the entity that DECLARES it, so + // an inherited attribute carries an ancestor's name. Prefixing the entity + // being mapped produced CE1613 "The selected attribute no longer exists" + // and left the field unmapped in Studio Pro (mendixlabs/mxcli#703) — the + // same rule as entity access rules (#758), both under the #765 umbrella. attr := def.Attribute if parentEntity != "" && !strings.Contains(attr, ".") { - attr = parentEntity + "." + attr + if ref, ok := ResolveMemberRef(b, parentEntity, attr); ok { + attr = ref + } else { + attr = parentEntity + "." + attr + } } elem.Attribute = attr } @@ -372,6 +381,18 @@ func resolveAttributeType(entityQN, attrName string, b backend.DomainModelBacken if len(parts) != 2 { return "String" } + // Follow the generalization chain: an inherited attribute is not in the + // entity's own list, and defaulting it to String gave a mapping element the + // wrong DataType (mendixlabs/mxcli#703). Resolving by module name also stops a + // same-named entity in another module being picked up, which the previous + // scan-every-domain-model loop did. + if hb, ok := b.(entityLookupBackend); ok { + if t := ResolveMemberType(hb, entityQN, attrName); t != "" { + return t + } + return "String" + } + dms, err := b.ListDomainModels() if err != nil { return "String" diff --git a/mdl/executor/entity_hierarchy.go b/mdl/executor/entity_hierarchy.go index 45f60dd63..f8cf05daa 100644 --- a/mdl/executor/entity_hierarchy.go +++ b/mdl/executor/entity_hierarchy.go @@ -6,6 +6,8 @@ import ( "sort" "strings" + "github.com/mendixlabs/mxcli/mdl/backend" + "github.com/mendixlabs/mxcli/sdk/domainmodel" ) @@ -49,6 +51,22 @@ type EntityMember struct { // members found so far are returned rather than nothing, so a partial model still // produces a usable rule. func EntityMembers(ctx *ExecContext, entityQN string) []EntityMember { + if ctx == nil { + return nil + } + return EntityMembersFor(ctx.Backend, entityQN) +} + +// entityLookupBackend is the slice of the backend the generalization walk needs: +// resolve a module by name, then read its domain model. +type entityLookupBackend interface { + backend.ModuleBackend + backend.DomainModelBackend +} + +// EntityMembersFor is EntityMembers against a backend directly, for callers that +// hold one without an ExecContext (the mapping builders). +func EntityMembersFor(b entityLookupBackend, entityQN string) []EntityMember { var out []EntityMember seen := map[string]bool{} // cycle guard claimed := map[string]bool{} // a child's member shadows the ancestor's @@ -65,7 +83,7 @@ func EntityMembers(ctx *ExecContext, entityQN string) []EntityMember { break } - entity, ok := findEntityByQN(ctx, currentQN) + entity, ok := findEntityByQN(b, currentQN) if !ok { break } @@ -87,20 +105,22 @@ func EntityMembers(ctx *ExecContext, entityQN string) []EntityMember { return out } -// findEntityByQN resolves a qualified entity name through the backend. -func findEntityByQN(ctx *ExecContext, entityQN string) (*domainmodel.Entity, bool) { - if ctx == nil || ctx.Backend == nil { +// findEntityByQN resolves a qualified entity name through the backend. The module +// is resolved by name rather than scanning every domain model, so a same-named +// entity in another module cannot be picked up by accident. +func findEntityByQN(b entityLookupBackend, entityQN string) (*domainmodel.Entity, bool) { + if b == nil { return nil, false } parts := strings.SplitN(entityQN, ".", 2) if len(parts) != 2 { return nil, false } - mod, err := ctx.Backend.GetModuleByName(parts[0]) + mod, err := b.GetModuleByName(parts[0]) if err != nil || mod == nil { return nil, false } - dm, err := ctx.Backend.GetDomainModel(mod.ID) + dm, err := b.GetDomainModel(mod.ID) if err != nil || dm == nil { return nil, false } @@ -128,3 +148,49 @@ func unmatchedGrantMembers(readMembers, writeMembers []string, granted map[strin sort.Strings(unknown) return unknown } + +// ResolveMemberRef returns the reference Mendix stores for a member of an entity, +// qualified against the entity that DECLARES it — which is an ancestor when the +// member is inherited. Reports false when the entity has no such member. +// +// Qualifying an inherited member against the entity that merely uses it produces +// CE1613 "The selected attribute ... no longer exists": in an access rule +// (mendixlabs/mxcli#758) and equally in an import/export mapping, where Studio Pro +// additionally shows the field as unmapped (#703). +func ResolveMemberRef(b entityLookupBackend, entityQN, memberName string) (string, bool) { + if entityQN == "" || memberName == "" { + return "", false + } + for _, mem := range EntityMembersFor(b, entityQN) { + if mem.Name == memberName { + return mem.Ref, true + } + } + return "", false +} + +// ResolveMemberType returns the data type of an entity's member, following the +// generalization chain. Returns "" when the member cannot be resolved. +func ResolveMemberType(b entityLookupBackend, entityQN, memberName string) string { + if entityQN == "" || memberName == "" { + return "" + } + seen := map[string]bool{} + for currentQN := entityQN; currentQN != ""; { + if seen[currentQN] { + return "" + } + seen[currentQN] = true + entity, ok := findEntityByQN(b, currentQN) + if !ok { + return "" + } + for _, attr := range entity.Attributes { + if attr != nil && attr.Name == memberName && attr.Type != nil { + return attr.Type.GetTypeName() + } + } + currentQN = entity.GeneralizationRef + } + return "" +} diff --git a/mdl/executor/entity_hierarchy_test.go b/mdl/executor/entity_hierarchy_test.go index d9b4b01a8..5b62767f1 100644 --- a/mdl/executor/entity_hierarchy_test.go +++ b/mdl/executor/entity_hierarchy_test.go @@ -45,6 +45,46 @@ func hierarchyBackend() *mock.MockBackend { } } +func attrTypeFor(kind string) domainmodel.AttributeType { + if kind == "Boolean" { + return &domainmodel.BooleanAttributeType{} + } + return &domainmodel.StringAttributeType{} +} + +// typedHierarchyBackend adds attribute types and an inherited Boolean, for the +// type-resolution tests. +func typedHierarchyBackend() *mock.MockBackend { + ids := map[string]model.ID{"App": "mod-app", "System": "mod-system"} + typed := func(name, kind string) *domainmodel.Attribute { + return &domainmodel.Attribute{Name: name, Type: attrTypeFor(kind)} + } + dms := map[model.ID]*domainmodel.DomainModel{ + ids["App"]: {ContainerID: ids["App"], Entities: []*domainmodel.Entity{ + {Name: "Base", Attributes: []*domainmodel.Attribute{ + typed("SharedField", "String"), typed("Flag", "Boolean")}}, + {Name: "Item", GeneralizationRef: "App.Base", Attributes: []*domainmodel.Attribute{ + typed("OwnField", "String")}}, + {Name: "Doc", GeneralizationRef: "System.FileDocument", Attributes: []*domainmodel.Attribute{ + typed("Category", "String")}}, + }}, + ids["System"]: {ContainerID: ids["System"], Entities: []*domainmodel.Entity{ + {Name: "FileDocument", Attributes: []*domainmodel.Attribute{typed("Name", "String")}}, + }}, + } + return &mock.MockBackend{ + IsConnectedFunc: func() bool { return true }, + GetModuleByNameFunc: func(name string) (*model.Module, error) { + id, ok := ids[name] + if !ok { + return nil, nil + } + return &model.Module{BaseElement: model.BaseElement{ID: id}, Name: name}, nil + }, + GetDomainModelFunc: func(id model.ID) (*domainmodel.DomainModel, error) { return dms[id], nil }, + } +} + func memberRefs(members []EntityMember) []string { out := make([]string, 0, len(members)) for _, m := range members { @@ -129,3 +169,65 @@ func TestUnmatchedGrantMembers(t *testing.T) { t.Errorf("unmatchedGrantMembers = %v, want [Alsobad Nope]", got) } } + +// TestResolveMemberRef_DeclaringEntity is the mapping half of the #765 umbrella +// (mendixlabs/mxcli#703): a mapping element bound to an inherited attribute was +// qualified against the entity being mapped, which is CE1613 "The selected +// attribute no longer exists" and leaves the field unmapped in Studio Pro. +func TestResolveMemberRef_DeclaringEntity(t *testing.T) { + b := typedHierarchyBackend() + + tests := []struct { + entity, member, want string + ok bool + }{ + {"App.Item", "OwnField", "App.Item.OwnField", true}, + {"App.Item", "SharedField", "App.Base.SharedField", true}, // inherited + {"App.Doc", "Name", "System.FileDocument.Name", true}, // inherited from System + {"App.Item", "Nonexistent", "", false}, + } + for _, tc := range tests { + got, ok := ResolveMemberRef(b, tc.entity, tc.member) + if ok != tc.ok || got != tc.want { + t.Errorf("ResolveMemberRef(%s, %s) = (%q, %v), want (%q, %v)", + tc.entity, tc.member, got, ok, tc.want, tc.ok) + } + } +} + +// TestResolveMemberType_FollowsChain: an inherited attribute's type was not found +// on the entity itself, so the mapping element defaulted to String — giving an +// inherited Boolean or DateTime the wrong DataType (#703). +func TestResolveMemberType_FollowsChain(t *testing.T) { + b := typedHierarchyBackend() + + if got := ResolveMemberType(b, "App.Item", "OwnField"); got != "String" { + t.Errorf("own attribute type = %q, want String", got) + } + if got := ResolveMemberType(b, "App.Item", "Flag"); got != "Boolean" { + t.Errorf("inherited attribute type = %q, want Boolean — "+ + "defaulting to String is what mistyped mapping elements", got) + } + if got := ResolveMemberType(b, "App.Item", "Nope"); got != "" { + t.Errorf("unresolvable member type = %q, want empty", got) + } +} + +// TestResolveAttributeType_InheritedAttribute covers the mapping call site rather +// than the resolver: resolveAttributeType scanned only the entity's own attributes +// and fell through to its "String" default, so a mapping element bound to an +// inherited Boolean or DateTime got the wrong DataType (mendixlabs/mxcli#703). +func TestResolveAttributeType_InheritedAttribute(t *testing.T) { + b := typedHierarchyBackend() + + if got := resolveAttributeType("App.Item", "OwnField", b); got != "String" { + t.Errorf("own attribute = %q, want String", got) + } + if got := resolveAttributeType("App.Item", "Flag", b); got != "Boolean" { + t.Errorf("inherited attribute = %q, want Boolean — the String default is the bug", got) + } + // An unresolvable member still falls back to the documented default. + if got := resolveAttributeType("App.Item", "Nope", b); got != "String" { + t.Errorf("unresolvable attribute = %q, want the String fallback", got) + } +} From d58bfbd48bf7249516c898fbebad2f117d928c5a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 06:54:08 +0000 Subject: [PATCH 5/6] test: give SecTest.Customer the Notes attribute its example grants The doctype example granted read on SecTest.Customer (Notes), but the entity only declares Name, Email and IsActive. Before #758 an unmatched member name was dropped in silence, so the grant did nothing and the script still passed; with that silence replaced by an error the example fails, and the integration tier caught it. The example is what is wrong: its own comment says "adding Notes access preserves existing Name and Email", so it always meant to demonstrate an additive grant on a third attribute. Declaring Notes makes it do that. Fixes the build-and-test failure on main introduced by #81. --- mdl-examples/doctype-tests/08-security-examples.mdl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/mdl-examples/doctype-tests/08-security-examples.mdl b/mdl-examples/doctype-tests/08-security-examples.mdl index 7f6395f16..7ef8b386b 100644 --- a/mdl-examples/doctype-tests/08-security-examples.mdl +++ b/mdl-examples/doctype-tests/08-security-examples.mdl @@ -34,7 +34,8 @@ create module SecTest; create persistent entity SecTest.Customer ( Name: string(200) not null, Email: string(200), - IsActive: boolean default true + IsActive: boolean default true, + Notes: string(500) ); @position(300,100) From d46d2ad04339286b0496a087c35eaa66c2f4e2af Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 2 Aug 2026 12:40:10 +0000 Subject: [PATCH 6/6] fix: write the Java version in the dialect the stored key expects The #759 fix followed the JavaVersion -> JavaMajorVersion key rename but wrote the caller's value through verbatim. The rename changed the value format too: 11.6 stores the enum member "Java21", 11.12 the bare major "21". So `alter settings model JavaVersion = 'Java21'` on an 11.12 project put "Java21" into JavaMajorVersion, and mxbuild refuses to load it: System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values. (Parameter 'majorVersion is an unsupported value: Java21') at Mendix.Modeler.Settings.JavaVersionExtensions.fromString This is a harder failure than the original #759 shape. That one wrote an unknown property, which mxbuild tolerates, so only Studio Pro broke; a wrong value for a known enum fails the whole project load, taking every check downstream of the settings unit with it. It is why the nightly went red on 14-project-settings-examples.mdl at 11.12 rather than surfacing as a user report. settingsoverlay.JavaVersionValue renders the value per stored key, so either spelling is accepted on input and stored in the project's own dialect. A value with no recognisable major version passes through untouched, so a typo surfaces as a Mendix error rather than as a silently mangled setting. Verified on real projects with mx check: 11.12.2 given 'Java21' stores '21' and checks clean on both engines (pre-fix binary reproduces the nightly error exactly); 11.6.6 given '21' stores 'Java21', 0 errors. Also routes the unused third copy in modelsdk/mpr/serialize_services.go through the same helper, so wiring it up later cannot reintroduce this. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012XR649rKk68z6gBpngu6MA --- .claude/skills/fix-issue.md | 1 + .claude/skills/mendix/project-settings.md | 11 +++- docs-site/src/language/project-settings.md | 5 ++ .../759-java-version-value-dialect.mdl | 32 +++++++++++ .../14-project-settings-examples.mdl | 4 ++ .../modelsdk/settings_write_759_test.go | 54 +++++++++++++++++++ mdl/settingsoverlay/settingsoverlay.go | 45 ++++++++++++++-- mdl/settingsoverlay/settingsoverlay_test.go | 47 ++++++++++++++++ modelsdk/mpr/serialize_services.go | 4 +- 9 files changed, 197 insertions(+), 6 deletions(-) create mode 100644 mdl-examples/bug-tests/759-java-version-value-dialect.mdl diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 5ff838e0d..0536a7267 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -319,6 +319,7 @@ cases for these three BSON types — they fell to `default: return nil`. | `UPDATE SECURITY`, `CREATE ASSOCIATION` or any `GRANT` silently strips **inherited** members from a specialized entity's access rules; `GRANT` naming an inherited member reports success and persists nothing, so REVOKE+GRANT cannot repair it. `mx check` shows only CE0066, hiding the CE2729 "No read access to attribute" cascade until Studio Pro's Update security is clicked | Mendix inheritance is multi-table: all of a parent's attributes are members of the child, so an access rule needs a MemberAccess entry for every member, own **and** inherited, each qualified against the entity that **declares** it. Both the GRANT builder and `ReconcileMemberAccesses` enumerated only `entity.Attributes`, so an inherited reference matched nothing and was deleted as stale — and reconciliation runs **immediately after every GRANT**, deleting what the grant had just written correctly | `mdl/executor/entity_hierarchy.go` (`EntityMembers`), `mdl/executor/cmd_security_write.go` (`execGrantEntityAccess`), `mdl/backend/modelsdk/domainmodel_security_write.go` (`ReconcileMemberAccesses`, `attrRefBelongsTo`), `sdk/mpr/writer_security.go` (legacy engine) | Walk the generalization chain and qualify each member against its declaring entity; in the reconciler, only strip a reference qualified to **this** entity — an ancestor may live in another module or System, neither loaded there, so preserve what cannot be validated. **Two facts must be established against `mx check`, never inferred**: (a) the child-qualified form is CE1613 "attribute no longer exists" while the declaring-entity form validates clean; (b) `System.User`'s members are the exception — entities specialising it are *user entities* whose platform members Mendix manages, and listing them turns a clean rule into CE0066, while omitting `System.FileDocument`'s six members is CE0066 until all are present. **Generalisable**: when a post-write reconcile pass validates against a narrower model than the writer used, it will quietly undo correct writes — check what runs *after* a write before concluding the writer is at fault. Repro `mdl-examples/bug-tests/758-inherited-member-access.mdl`. Issues #758, #765 (umbrella; #451 is the same declaring-entity rule in the change-object writer) | | `describe` (and `context` / `diff-local`) renders a Retrieve's XPath with only its **first** predicate group — `where A/B[EndDate = $X];` when the BSON holds `[A/B[EndDate = $X]][Status != 'Completed'][CompletionDate = empty]`. No warning; the output reads as a complete but materially *less restrictive* query, so correct defensive code looks buggy | The grammar's `xpathConstraint` rule matches ONE bracket group, and Mendix concatenates siblings. `ParseXPathConstraint` removes the error listeners, so ANTLR parsed group 1, left the rest on the token stream, and **still returned ok=true**; `enrichXPathConstraintForDescribe` treated that as a full parse and re-rendered only what came back. The `if !ok { return original }` fallback never fired | `mdl/visitor/visitor_xpath_public.go` (`ParseXPathConstraint`), `mdl/visitor/xpath_groups.go` (`SplitXPathPredicateGroups`), `mdl/executor/cmd_microflows_format_action.go` (`enrichXPathGroups`, and the render-path split) | Two layers. (1) Reject a partial parse — after the rule, require `stream.LA(1) == antlr.TokenEOF`; that alone stops the loss, since the caller then falls back to the stored string. (2) Split into top-level groups and enrich each, so enrichment still reaches groups after the first. The splitter must track **nesting depth and quoting**: a naive `][` split mangles a nested `[A/B[x = 1]]` and a literal containing `]`. **Generalisable**: a parser that silently accepts a prefix is worse than one that fails — any `ok` returned by a rule that can match less than its input must be checked against EOF before callers treat it as lossless. Repro `mdl-examples/bug-tests/772-xpath-constraint-groups.mdl`; A/B against a pre-fix binary on the same project shows the two dropped groups. Issue #772 | | An import/export mapping over an entity created with `EXTENDS` maps only its **own** attributes; every inherited field shows unmapped in Studio Pro, and `mx check` reports CE1613 "The selected attribute 'Mod.Child.Attr' no longer exists". An inherited Boolean/DateTime element also gets `DataType=String` | The mapping builder prefixed the entity being mapped unconditionally (`attr = parentEntity + "." + attr`), but a member reference is qualified against the entity that **declares** it — the same rule as entity access rules (#758) and the change-object writer (#451). Separately `resolveAttributeType` scanned only the entity's own attributes and fell through to its `"String"` default | `mdl/executor/cmd_import_mappings.go` and `cmd_export_mappings.go` (both carry the same two lines), `mdl/executor/entity_hierarchy.go` (`ResolveMemberRef`, `ResolveMemberType`) | Route both sites through the generalization walk added for #758: `ResolveMemberRef` returns the declaring-entity reference and `ResolveMemberType` finds the type up the chain, each falling back to the old behaviour when the member cannot be resolved. **Watch for the sibling defect**: the old `resolveAttributeType` matched entities **by name across every domain model**, so a same-named entity in another module could win — resolve the module by name instead. **Generalisable**: when one rule has several call sites, a fix at one of them proves nothing about the others; grep for the *pattern* (`range entity.Attributes`, `parentEntity + "."`) rather than the reported symptom. Repro `mdl-examples/bug-tests/703-mapping-inherited-attributes.mdl`; A/B on the same project shows `Map703.Contract.DocName` (CE1613) become `Map703.DocumentBase.DocName`. Issue #703, umbrella #765 | +| `alter settings model JavaVersion = 'Java21'` on Mendix 11.12+ produces a project mxbuild refuses to **load**: `mx check` reports `System.ArgumentOutOfRangeException ... (Parameter 'majorVersion is an unsupported value: Java21')` at `JavaVersionExtensions.fromString`. Every check downstream of the settings unit is lost with it | Mendix renamed the property between 11.6 (`JavaVersion` = `"Java21"`) and 11.12 (`JavaMajorVersion` = `"21"`) — and the rename changed the **value format** as well as the key. The #759 fix followed only the key, writing the caller's value through verbatim, so the 11.6 spelling landed in the 11.12 key | `mdl/settingsoverlay/settingsoverlay.go` (`JavaVersionValue`, `SetJavaVersion`) — shared by both engines; the dead third copy in `modelsdk/mpr/serialize_services.go` carried it too | Render the value in the dialect the stored key expects: strip/add the `Java` prefix per key, and pass an unrecognisable value through untouched so a typo surfaces as a Mendix error instead of a mangled setting. **Generalisable**: a renamed property is not only a renamed key — check whether the value encoding moved with it, and cover *both* directions (either spelling in, document's dialect out). Note the sharper failure mode: the original #759 shape was an unknown property, which mxbuild **tolerates**, so only Studio Pro broke; a wrong *value* for a known enum is a hard build failure, which is why this one surfaced as a red nightly rather than a user report. Repro `mdl-examples/bug-tests/759-java-version-value-dialect.mdl`. Issue #759 (follow-up) | **Key insight:** `microflows$ListRange` stores offset/limit inside a nested `CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before diff --git a/.claude/skills/mendix/project-settings.md b/.claude/skills/mendix/project-settings.md index f6a10b2e8..ce601688f 100644 --- a/.claude/skills/mendix/project-settings.md +++ b/.claude/skills/mendix/project-settings.md @@ -30,12 +30,21 @@ alter settings model BeforeShutdownMicroflow = 'Module.MF_Shutdown'; alter settings model HealthCheckMicroflow = 'Module.MF_HealthCheck'; alter settings model HashAlgorithm = 'BCrypt'; alter settings model BcryptCost = 12; -alter settings model JavaVersion = 'Java21'; +alter settings model JavaVersion = 'Java21'; -- or '21'; see note below alter settings model RoundingMode = 'HalfUp'; alter settings model AllowUserMultipleSessions = true; alter settings model ScheduledEventTimeZoneCode = 'Etc/UTC'; ``` +**JavaVersion spelling.** Mendix renamed this property between versions: up to 11.6 +it stores `JavaVersion` = `'Java21'`, from 11.12 it stores `JavaMajorVersion` = +`'21'`. Write either spelling — mxcli reads which one the project uses and stores +the value in that dialect. Getting this wrong is not a cosmetic difference: 11.12 +parses the bare major and rejects the project outright with +`ArgumentOutOfRangeException: majorVersion is an unsupported value: Java21`. +`describe settings` always emits the project's own spelling, so its output replays +cleanly. + ### Modify Configuration Settings ```sql diff --git a/docs-site/src/language/project-settings.md b/docs-site/src/language/project-settings.md index 5ecd76212..3e5c58103 100644 --- a/docs-site/src/language/project-settings.md +++ b/docs-site/src/language/project-settings.md @@ -32,6 +32,11 @@ ALTER SETTINGS MODEL HashAlgorithm = 'BCrypt'; ALTER SETTINGS MODEL JavaVersion = '17'; ``` +Mendix renamed the Java version property between versions — up to 11.6 it is stored +as `JavaVersion` = `Java21`, from 11.12 as `JavaMajorVersion` = `21`. Write either +spelling (`'17'` or `'Java17'`): mxcli stores the value in whichever dialect the +project already uses. + ### Configuration Settings Server configuration settings like database type, URL, and HTTP port. Each configuration is identified by name (commonly `'default'`): diff --git a/mdl-examples/bug-tests/759-java-version-value-dialect.mdl b/mdl-examples/bug-tests/759-java-version-value-dialect.mdl new file mode 100644 index 000000000..423684f33 --- /dev/null +++ b/mdl-examples/bug-tests/759-java-version-value-dialect.mdl @@ -0,0 +1,32 @@ +-- Bug #759 (follow-up): the Java version *value* format is version-specific too +-- +-- Symptom: on Mendix 11.12 an `alter settings model JavaVersion = 'Java21'` +-- produced a project mx check refuses to load at all: +-- +-- ERROR: System.ArgumentOutOfRangeException: Specified argument was out of the +-- range of valid values. (Parameter 'majorVersion is an unsupported value: Java21') +-- at Mendix.Modeler.Settings.JavaVersionExtensions.fromString(String majorVersion) +-- at Mendix.Modeler.Settings.RuntimeSettings.get_JavaVersion() +-- +-- Cause: the #759 fix followed the *key* rename ("JavaVersion" up to 11.6, +-- "JavaMajorVersion" from 11.12) but wrote the value through verbatim. The rename +-- changed the value format as well: 11.6 stores the enum member "Java21", 11.12 +-- the bare major "21". Writing "Java21" into JavaMajorVersion is what +-- JavaVersionExtensions.fromString throws on. +-- +-- Before the fix this was worse than a wrong value: unlike the unknown-property +-- shape of the original #759, this one is a *hard* mxbuild failure — the whole +-- project fails to load, so nothing downstream of it can be checked either. +-- +-- Fix: settingsoverlay.JavaVersionValue renders the value in the dialect the +-- stored key expects. Either spelling is accepted on input. +-- +-- Verify: run against an 11.12+ project, then `mx check` — it must load and +-- report no settings error. On an 11.6 project the same script stores 'Java21'. + +-- Both spellings are accepted; both land as the project's own dialect. +alter settings model JavaVersion = 'Java21'; +alter settings model JavaVersion = '21'; + +-- Round-trip: describe emits the project's spelling, which must replay cleanly. +describe settings; diff --git a/mdl-examples/doctype-tests/14-project-settings-examples.mdl b/mdl-examples/doctype-tests/14-project-settings-examples.mdl index 25520c5a9..5d59cbac0 100644 --- a/mdl-examples/doctype-tests/14-project-settings-examples.mdl +++ b/mdl-examples/doctype-tests/14-project-settings-examples.mdl @@ -36,6 +36,10 @@ alter settings model AfterStartupMicroflow = 'MyModule.ASU_Startup'; /** * Example 1.2: Configure hash algorithm and Java version + * + * Either spelling of the Java version is accepted -- 'Java21' or '21'. Mendix + * stores it as "JavaVersion" = "Java21" up to 11.6 and as "JavaMajorVersion" = + * "21" from 11.12; mxcli writes whichever dialect the project already uses. */ alter settings model HashAlgorithm = 'BCrypt', JavaVersion = 'Java21'; diff --git a/mdl/backend/modelsdk/settings_write_759_test.go b/mdl/backend/modelsdk/settings_write_759_test.go index 51a475cca..ba9e6a5ee 100644 --- a/mdl/backend/modelsdk/settings_write_759_test.go +++ b/mdl/backend/modelsdk/settings_write_759_test.go @@ -97,6 +97,60 @@ func TestUpdateProjectSettings_JavaVersionKeyFollowsDocument(t *testing.T) { } } +// TestUpdateProjectSettings_JavaVersionValueMatchesKey covers the follow-up to +// #759: the rename changed the value format along with the key. Mendix 11.12 +// parses JavaMajorVersion with JavaVersionExtensions.fromString, which throws +// ArgumentOutOfRangeException ("majorVersion is an unsupported value: Java21") on +// the 11.6 spelling — so writing the value through verbatim produced a project +// mx check refuses to load. Either spelling on input, the document's own dialect +// on disk. +func TestUpdateProjectSettings_JavaVersionValueMatchesKey(t *testing.T) { + tests := []struct { + name string + storedKey string + seed string + set string + want string + }{ + {"11_12_given_enum_spelling", "JavaMajorVersion", "21", "Java17", "17"}, + {"11_12_given_bare_major", "JavaMajorVersion", "21", "17", "17"}, + {"11_6_given_enum_spelling", "JavaVersion", "Java21", "Java17", "Java17"}, + {"11_6_given_bare_major", "JavaVersion", "Java21", "17", "Java17"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + proj := copyFixture(t) + seedJavaVersionKey(t, proj, tc.storedKey, tc.seed) + + setJavaVersion(t, proj, tc.set) + + if got := readModelSettings(t, proj)[tc.storedKey]; got != tc.want { + t.Errorf("%s = %v, want %q (set %q)", tc.storedKey, got, tc.want, tc.set) + } + }) + } +} + +// setJavaVersion drives one ALTER SETTINGS MODEL JavaVersion through the backend. +func setJavaVersion(t *testing.T, proj, v string) { + t.Helper() + b := New() + if err := b.Connect(proj); err != nil { + t.Fatalf("connect: %v", err) + } + ps, err := b.GetProjectSettings() + if err != nil { + t.Fatalf("GetProjectSettings: %v", err) + } + if ps.Model == nil { + t.Fatal("fixture has no model settings") + } + ps.Model.JavaVersion = v + if err := b.UpdateProjectSettings(ps); err != nil { + t.Fatalf("UpdateProjectSettings: %v", err) + } +} + // seedJavaVersionKey rewrites the fixture's Settings$ModelSettings part so it // carries exactly one Java-version key, standing in for the Mendix version that // spells it that way. diff --git a/mdl/settingsoverlay/settingsoverlay.go b/mdl/settingsoverlay/settingsoverlay.go index a3a7c52a1..615eafe5f 100644 --- a/mdl/settingsoverlay/settingsoverlay.go +++ b/mdl/settingsoverlay/settingsoverlay.go @@ -134,6 +134,13 @@ func newServerConfiguration(cfg *model.ServerConfiguration, siblings []map[strin } } +// The two spellings of the runtime Java version property. They differ in value +// format as well as in name — see JavaVersionValue. +const ( + JavaMajorVersionKey = "JavaMajorVersion" // Mendix 11.12+, e.g. "21" + JavaVersionEnumKey = "JavaVersion" // Mendix 11.6, e.g. "Java21" +) + // JavaVersionKey returns the storage key a Settings$ModelSettings part uses for // the runtime Java version, or "" when it carries neither. // @@ -146,7 +153,7 @@ func newServerConfiguration(cfg *model.ServerConfiguration, siblings []map[strin // (mendixlabs/mxcli#759). Read the key off the document instead of assuming one, // and never invent a key the document does not already have. func JavaVersionKey(raw map[string]any) string { - for _, k := range []string{"JavaMajorVersion", "JavaVersion"} { + for _, k := range []string{JavaMajorVersionKey, JavaVersionEnumKey} { if _, ok := raw[k]; ok { return k } @@ -165,12 +172,42 @@ func JavaVersion(raw map[string]any) string { return v } -// SetJavaVersion writes the runtime Java version back to the key it was read from. -// A part carrying neither key is left untouched. +// SetJavaVersion writes the runtime Java version back to the key it was read from, +// in the value format that key expects. A part carrying neither key is left +// untouched. func SetJavaVersion(raw map[string]any, v string) { if k := JavaVersionKey(raw); k != "" { - raw[k] = v + raw[k] = JavaVersionValue(k, v) + } +} + +// JavaVersionValue renders a Java version in the form the given storage key holds: +// "JavaVersion" carries the enum member ("Java21"), "JavaMajorVersion" the bare +// major ("21"). +// +// The rename in #759 changed the value format along with the key, and following +// only the key is not enough: Mendix 11.12 parses JavaMajorVersion with +// JavaVersionExtensions.fromString, which throws ArgumentOutOfRangeException +// ("majorVersion is an unsupported value: Java21") on the 11.6 spelling. So an +// `alter settings model JavaVersion = 'Java21'` written verbatim onto an 11.12 +// document produces a project mx check refuses to load. Either spelling is +// accepted on input and stored in the document's own dialect. +// +// A value that is neither spelling — no recognisable major version — is passed +// through untouched, so a typo surfaces as a Mendix error rather than as a +// silently mangled setting. +func JavaVersionValue(key, v string) string { + major := strings.TrimSpace(v) + if len(major) >= 4 && strings.EqualFold(major[:4], "Java") { + major = major[4:] + } + if major == "" || strings.TrimLeft(major, "0123456789") != "" { + return v + } + if key == JavaMajorVersionKey { + return major } + return "Java" + major } // ConstantValues rebuilds a configuration's ConstantValues list, updating each diff --git a/mdl/settingsoverlay/settingsoverlay_test.go b/mdl/settingsoverlay/settingsoverlay_test.go index eef02b4d9..989ab1498 100644 --- a/mdl/settingsoverlay/settingsoverlay_test.go +++ b/mdl/settingsoverlay/settingsoverlay_test.go @@ -365,3 +365,50 @@ func TestJavaVersionKey_FollowsDocument(t *testing.T) { }) } } + +// TestJavaVersionValue_MatchesKeyDialect is the follow-up to +// TestJavaVersionKey_FollowsDocument: the rename changed the value format along +// with the key, so following only the key still produced a project Mendix 11.12 +// refuses to load. Its JavaVersionExtensions.fromString parses JavaMajorVersion as +// a bare major and throws ArgumentOutOfRangeException ("majorVersion is an +// unsupported value: Java21") on the 11.6 spelling. +func TestJavaVersionValue_MatchesKeyDialect(t *testing.T) { + tests := []struct { + key string + in string + want string + }{ + {JavaMajorVersionKey, "Java21", "21"}, + {JavaMajorVersionKey, "21", "21"}, + {JavaMajorVersionKey, "java17", "17"}, + {JavaVersionEnumKey, "Java21", "Java21"}, + {JavaVersionEnumKey, "21", "Java21"}, + {JavaVersionEnumKey, " 17 ", "Java17"}, + // Not a recognisable version: passed through so the typo surfaces as a + // Mendix error rather than as a silently mangled setting. + {JavaMajorVersionKey, "Temurin", "Temurin"}, + {JavaVersionEnumKey, "Java-21", "Java-21"}, + {JavaMajorVersionKey, "", ""}, + } + for _, tc := range tests { + if got := JavaVersionValue(tc.key, tc.in); got != tc.want { + t.Errorf("JavaVersionValue(%q, %q) = %q, want %q", tc.key, tc.in, got, tc.want) + } + } +} + +// TestSetJavaVersion_ConvertsToStoredDialect: one MDL statement must work on both +// Mendix versions, whichever spelling the author used. +func TestSetJavaVersion_ConvertsToStoredDialect(t *testing.T) { + mendix1112 := map[string]any{JavaMajorVersionKey: "21"} + SetJavaVersion(mendix1112, "Java17") + if got := mendix1112[JavaMajorVersionKey]; got != "17" { + t.Errorf("%s = %#v, want %q", JavaMajorVersionKey, got, "17") + } + + mendix116 := map[string]any{JavaVersionEnumKey: "Java21"} + SetJavaVersion(mendix116, "17") + if got := mendix116[JavaVersionEnumKey]; got != "Java17" { + t.Errorf("%s = %#v, want %q", JavaVersionEnumKey, got, "Java17") + } +} diff --git a/modelsdk/mpr/serialize_services.go b/modelsdk/mpr/serialize_services.go index 7f493b4bc..a2cb94f4d 100644 --- a/modelsdk/mpr/serialize_services.go +++ b/modelsdk/mpr/serialize_services.go @@ -7,6 +7,7 @@ import ( "go.mongodb.org/mongo-driver/v2/bson" + "github.com/mendixlabs/mxcli/mdl/settingsoverlay" "github.com/mendixlabs/mxcli/mdl/types" "github.com/mendixlabs/mxcli/model" ) @@ -191,7 +192,8 @@ func serPSModelSettings(ms *model.ModelSettings, raw map[string]any) map[string] raw["AllowUserMultipleSessions"] = ms.AllowUserMultipleSessions raw["HashAlgorithm"] = ms.HashAlgorithm raw["BcryptCost"] = serPSInt64(ms.BcryptCost) - raw["JavaVersion"] = ms.JavaVersion + // Version-specific key AND value format — see settingsoverlay.JavaVersionValue. + settingsoverlay.SetJavaVersion(raw, ms.JavaVersion) raw["RoundingMode"] = ms.RoundingMode raw["ScheduledEventTimeZoneCode"] = ms.ScheduledEventTimeZoneCode raw["FirstDayOfWeek"] = ms.FirstDayOfWeek