From 4156a25018ebaf96887705123bb1de4dc6037335 Mon Sep 17 00:00:00 2001 From: Joshua de Haan Date: Thu, 12 Nov 2026 17:38:53 +0100 Subject: [PATCH] fix: check access rule constraints with a token MxBuild rejects (CE0161) (closes #1843) literalTypeProblem judged '[%CurrentUser%]' and returned early for every other token, and literalTypeProblems only looked up attributes, so these passed check -p and exec and failed mx check 11.14.0 (Production) with CE0161 "Error(s) in XPath constraint.": - a quoted token on a number ([Cnt = '[%DayLength%]']): now refused like '5'; - a date token or a [%UserRole_...%] token on a String; - a [%UserRole_...%] token on a DateTime; - '[%CurrentUser%]' compared with an association whose far end is not System.User or a specialization of it. A date token on a DateTime and '[%CurrentUser%]' on an association to System.User or Administration.Account stay silent, as measured. Tokens on a Boolean or enumeration, other tokens on a String or DateTime, an association the entity is not the FROM end of, and a far end whose generalization chain cannot be read stay silent. Co-Authored-By: Claude Opus 5.5 --- ...11-12-access-rule-xpath-tokens-ce0161.json | 1 + mdl/executor/validate_access_rule_xpath.go | 187 ++++++++++++++++-- .../validate_access_rule_xpath_test.go | 21 +- mdl/executor/validate_grant_refs.go | 3 + 4 files changed, 192 insertions(+), 20 deletions(-) create mode 100644 .claude/skills/fix-issue/findings/mdl-executor/2026-11-12-access-rule-xpath-tokens-ce0161.json diff --git a/.claude/skills/fix-issue/findings/mdl-executor/2026-11-12-access-rule-xpath-tokens-ce0161.json b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-12-access-rule-xpath-tokens-ce0161.json new file mode 100644 index 000000000..6fc097dfc --- /dev/null +++ b/.claude/skills/fix-issue/findings/mdl-executor/2026-11-12-access-rule-xpath-tokens-ce0161.json @@ -0,0 +1 @@ +{"area": "mdl/executor", "date": "2026-11-12", "refs": ["mendixlabs/mxcli#1843", "mendixlabs/mxcli#1803"], "ce": ["CE0161"], "rules": [], "symptom": "An access rule WHERE comparing an attribute with a token other than [%CurrentUser%], or an association with '[%CurrentUser%]', passed check -p and exec and failed mx check (11.14.0, Production) with CE0161 \"Error(s) in XPath constraint.\": [Title = '[%CurrentDateTime%]'], [Title = '[%UserRole_User%]'], [D = '[%UserRole_User%]'], [Cnt = '[%DayLength%]'], [M.E_Kid = '[%CurrentUser%]'] where M.Kid is not a user.", "file": "mdl/executor/validate_access_rule_xpath.go (literalTypeProblem: xpathToken, tokenTypeProblem; literalTypeProblems: walkXPathMemberComparisons, currentUserAssociationProblem); test validate_access_rule_xpath_test.go", "cause": "literalTypeProblem returned early for every token but [%CurrentUser%], and literalTypeProblems only looked up attributes, so a qualified association compared with a literal was never judged.", "insight": "A quoted token is a string literal to mxbuild's type check: on a number it is refused like '5'. A date token is a DateTime and a [%UserRole_…%] token a user role; '[%CurrentUser%]' takes an association whose far end is System.User or a specialization of it (Administration.Account builds)."} diff --git a/mdl/executor/validate_access_rule_xpath.go b/mdl/executor/validate_access_rule_xpath.go index a50ecb816..ce23aeacb 100644 --- a/mdl/executor/validate_access_rule_xpath.go +++ b/mdl/executor/validate_access_rule_xpath.go @@ -51,12 +51,27 @@ import ( // runs too. The association and System-member rules are the retrieve's own // functions, not a third copy. // +// Tokens other than [%CurrentUser%], measured the same way: a quoted token is +// a string, so any token on a number is CE0161, like '5'; a date token +// ([%CurrentDateTime%], [%BeginOf…%], [%EndOf…%]) or a user role token +// ([%UserRole_…%]) on a String is CE0161; a user role token on a DateTime is +// CE0161, a date token on one builds. And '[%CurrentUser%]' compared with an +// association whose far end is not System.User or a specialization of it is +// CE0161. +// +// CE0161 [Title = '[%CurrentDateTime%]'] [Title = '[%BeginOfCurrentDay%]'] +// [Title = '[%UserRole_User%]'] [D = '[%UserRole_User%]'] +// [Cnt = '[%CurrentDateTime%]'] [Cnt = '[%DayLength%]'] +// [M.E_Kid = '[%CurrentUser%]'] (M.Kid is not a user) +// 0 errors [M.E_Acc = '[%CurrentUser%]'] (Administration.Account, a +// specialization of System.User) +// // Could-not-establish is silence: an attribute reached over a path, an // inherited one, one of a kind not measured (AutoNumber, HashedString, Binary, -// Date), an entity the script alters, a token other than [%CurrentUser%] (a -// date token compared with a number was measured as CE0161 too, but the token -// vocabulary is not modelled here), and an enumeration whose values neither the -// script nor the project states. +// Date), an entity the script alters, a token on a Boolean or enumeration, any +// other token on a String or DateTime, an association the entity is not the +// FROM end of, a far end whose generalization chain cannot be read to the root, +// and an enumeration whose values neither the script nor the project states. // accessRuleVariableRule reports a variable in an access rule. A flow, a page // and a user task have variables to hand an XPath constraint; an access rule @@ -181,37 +196,56 @@ func (l xpathLiteral) text() string { // walkXPathLiteralComparisons calls fn for each comparison of a bare attribute // with a literal, either way round, through and/or, not() and parentheses. func walkXPathLiteralComparisons(e ast.Expression, fn func(attr, op string, lit xpathLiteral)) { + walkXPathMemberComparisons(e, func(member ast.Expression, op string, lit xpathLiteral) { + if id, ok := member.(*ast.IdentifierExpr); ok { + fn(id.Name, op, lit) + } + }) +} + +// walkXPathMemberComparisons calls fn for each comparison of a member — a bare +// attribute or a qualified association — with a literal, either way round, +// through and/or, not() and parentheses. +func walkXPathMemberComparisons(e ast.Expression, fn func(member ast.Expression, op string, lit xpathLiteral)) { switch x := e.(type) { case *ast.ParenExpr: - walkXPathLiteralComparisons(x.Inner, fn) + walkXPathMemberComparisons(x.Inner, fn) case *ast.UnaryExpr: - walkXPathLiteralComparisons(x.Operand, fn) + walkXPathMemberComparisons(x.Operand, fn) case *ast.FunctionCallExpr: if strings.EqualFold(x.Name, "not") { for _, a := range x.Arguments { - walkXPathLiteralComparisons(a, fn) + walkXPathMemberComparisons(a, fn) } } case *ast.BinaryExpr: op := strings.ToLower(x.Operator) switch op { case "and", "or": - walkXPathLiteralComparisons(x.Left, fn) - walkXPathLiteralComparisons(x.Right, fn) + walkXPathMemberComparisons(x.Left, fn) + walkXPathMemberComparisons(x.Right, fn) case "=", "!=", "<>", "<", ">", "<=", ">=": - if id, ok := x.Left.(*ast.IdentifierExpr); ok { + if isXPathMember(x.Left) { if lit, ok := xpathLiteralOf(x.Right); ok { - fn(id.Name, op, lit) + fn(x.Left, op, lit) } - } else if id, ok := x.Right.(*ast.IdentifierExpr); ok { + } else if isXPathMember(x.Right) { if lit, ok := xpathLiteralOf(x.Left); ok { - fn(id.Name, op, lit) + fn(x.Right, op, lit) } } } } } +func isXPathMember(e ast.Expression) bool { + switch e.(type) { + case *ast.IdentifierExpr, *ast.QualifiedNameExpr: + return true + } + return false +} + // literalTypeProblems reports each literal in the constraint whose type the // attribute it is compared with does not take (the table in the file comment). func (g *grantResolver) literalTypeProblems(entityQN, constraint string) []string { @@ -226,7 +260,15 @@ func (g *grantResolver) literalTypeProblems(entityQN, constraint string) []strin if !ok || expr == nil { continue } - walkXPathLiteralComparisons(expr, func(attr, op string, lit xpathLiteral) { + walkXPathMemberComparisons(expr, func(member ast.Expression, op string, lit xpathLiteral) { + if qn, ok := member.(*ast.QualifiedNameExpr); ok { + if p := g.currentUserAssociationProblem(entityQN, qn.QualifiedName.String(), op, lit); p != "" && !seen[p] { + seen[p] = true + out = append(out, p) + } + return + } + attr := member.(*ast.IdentifierExpr).Name at, ok := retrieveAttributeType(g.ctx, g.whole, entityQN, attr) if !ok { return @@ -257,8 +299,10 @@ func (g *grantResolver) literalTypeProblem(at ast.DataType, op string, lit xpath return "[%CurrentUser%] is the current user object, which compares with an association to System.User " + "(or System.owner), not with an attribute value" } - if lit.class == "string" && strings.Contains(lit.value, "[%") { - return "" // another token: its type is not modelled here + if tok, ok := xpathToken(lit); ok && class != "number" { + // On a number every quoted value is refused, a token included (the + // number case below). Elsewhere only the measured pairs are judged. + return tokenTypeProblem(class, tok) } switch class { case "string": @@ -321,3 +365,114 @@ func (g *grantResolver) enumValues(enumQN string) ([]string, bool) { } return v, true } + +// xpathToken is the name of the token a string literal is, such as +// "CurrentDateTime" for '[%CurrentDateTime%]'. +func xpathToken(lit xpathLiteral) (string, bool) { + if lit.class != "string" || !strings.HasPrefix(lit.value, "[%") || !strings.HasSuffix(lit.value, "%]") { + return "", false + } + return strings.TrimSuffix(strings.TrimPrefix(lit.value, "[%"), "%]"), true +} + +// isDateToken reports the tokens that stand for a date and time. +func isDateToken(tok string) bool { + return tok == "CurrentDateTime" || strings.HasPrefix(tok, "BeginOf") || strings.HasPrefix(tok, "EndOf") +} + +// tokenTypeProblem is why an attribute of the class does not take the token +// (CurrentUser is judged before), or "" when it does or that was not measured. +func tokenTypeProblem(class, tok string) string { + userRole := strings.HasPrefix(tok, "UserRole_") + switch class { + case "string": + if isDateToken(tok) { + return "[%" + tok + "%] is a date and time, not a string; it compares with a DateTime attribute" + } + if userRole { + return "[%" + tok + "%] is a user role, not a string" + } + case "datetime": + if userRole { + return "[%" + tok + "%] is a user role, not a date; a DateTime attribute compares with a date string or a date token such as '[%CurrentDateTime%]'" + } + } + return "" +} + +// currentUserAssociationProblem reports '[%CurrentUser%]' compared with an +// association of the entity whose far end is not System.User or a +// specialization of it. Silent unless the association is known, the entity is +// its FROM end, and the far end's generalization chain was read to the root. +func (g *grantResolver) currentUserAssociationProblem(entityQN, assocQN, op string, lit xpathLiteral) string { + if tok, ok := xpathToken(lit); !ok || tok != "CurrentUser" || (op != "=" && op != "!=") { + return "" + } + from, to, ok := g.associationEndsFor(assocQN) + if !ok || !strings.EqualFold(from, entityQN) { + return "" + } + chain, complete := g.generalizationChain(to) + if !complete { + return "" + } + for _, e := range chain { + if strings.EqualFold(e, "System.User") { + return "" + } + } + return fmt.Sprintf("the access rule's constraint compares association %s, which leads to %s, with '[%%CurrentUser%%]' — "+ + "the current user is a System.User and %s is not one; mxbuild rejects the constraint with CE0161 \"Error(s) in XPath constraint.\"", + assocQN, to, to) +} + +// associationEndsFor is an association's FROM and TO entity, as the script +// declares it or else as the project stores it. +func (g *grantResolver) associationEndsFor(assocQN string) (string, string, bool) { + if ends, ok := g.whole.associationEnds[strings.ToLower(assocQN)]; ok { + return ends[0], ends[1], true + } + if g.assocCatalog == nil { + var cat associationCatalog + if b, ok := g.ctx.Backend.(entityLookupBackend); ok { + cat = associationCatalogFor(b) + } + g.assocCatalog = &cat + } + shape, ok := g.assocCatalog.shapes[strings.ToLower(assocQN)] + if !ok { + return "", "", false + } + return shape.from, shape.to, true +} + +// generalizationChain is the entity followed by its generalizations, nearest +// first, and whether the walk reached the root. An entity the script declares +// answers with its declared generalization; one it alters, renames or moves is +// not established. +func (g *grantResolver) generalizationChain(entityQN string) ([]string, bool) { + b, _ := g.ctx.Backend.(entityLookupBackend) + var chain []string + seen := map[string]bool{} + for qn := entityQN; qn != ""; { + key := strings.ToLower(qn) + if seen[key] || g.whole.alteredEntities[qn] || g.whole.indirectEntities[qn] { + return chain, false + } + seen[key] = true + chain = append(chain, qn) + if gen, ok := g.whole.entityGeneralizations[key]; ok { + qn = gen + continue + } + if strings.EqualFold(qn, "System.User") { + return chain, true + } + e, ok := findEntityByQN(b, qn) + if !ok { + return chain, false + } + qn = e.GeneralizationRef + } + return chain, true +} diff --git a/mdl/executor/validate_access_rule_xpath_test.go b/mdl/executor/validate_access_rule_xpath_test.go index b4cb87d16..0f358afbb 100644 --- a/mdl/executor/validate_access_rule_xpath_test.go +++ b/mdl/executor/validate_access_rule_xpath_test.go @@ -27,6 +27,8 @@ create persistent entity M.E ( Title: String(100), Cnt: Integer, Big: Long, Amt: create persistent entity M.O ( Title: String(100), owner: AutoOwner ); create association M.E_Kid from M.E to M.Kid; create association M.E_User from M.E to System.User; +create persistent entity M.Acc extends System.User ( Nick: String(50) ); +create association M.E_Acc from M.E to M.Acc; create module role M.User; ` @@ -95,6 +97,15 @@ func TestAccessRuleXPath_ReportsWhatMxbuildRejects(t *testing.T) { {"empty key", "[Status = '']", "Red, Grn"}, {"number on an enumeration", "[Status = 5]", "M.Color"}, {"inside an and", "[Title = 'a' and Cnt = '5']", "Integer"}, + // tokens other than CurrentUser + {"date token on a String", "[Title = '[%CurrentDateTime%]']", "[%CurrentDateTime%]"}, + {"begin-of token on a String", "[Title = '[%BeginOfCurrentDay%]']", "[%BeginOfCurrentDay%]"}, + {"user role token on a String", "[Title = '[%UserRole_User%]']", "[%UserRole_User%]"}, + {"user role token on a DateTime", "[D = '[%UserRole_User%]']", "[%UserRole_User%]"}, + {"date token on an Integer", "[Cnt = '[%CurrentDateTime%]']", "Integer"}, + {"quoted number token on an Integer", "[Cnt = '[%DayLength%]']", "Integer"}, + // the current user compared with an association to a non-user + {"user token on an association to M.Kid", "[M.E_Kid = '[%CurrentUser%]']", "M.Kid is not one"}, } ctx := accessRuleCtx(t) for _, c := range cases { @@ -138,11 +149,13 @@ func TestAccessRuleXPath_LegalTwinsPass(t *testing.T) { {"M.E", "[Cnt = empty]"}, {"M.E", "[Status = empty]"}, {"M.E", "[Status = 'Red' and Cnt = 5 and Flag = 'false' and D = '2020-01-01' and Title = 'x']"}, - // Not established, so not judged: a token other than CurrentUser on a - // number (measured CE0161, but the token vocabulary is not modelled), - // and an attribute reached over a path. - {"M.E", "[Cnt = '[%DayLength%]']"}, + // the current user compared with an association to a specialization + // of System.User + {"M.E", "[M.E_Acc = '[%CurrentUser%]']"}, + // Not established, so not judged: an attribute reached over a path, + // and a token on a String that was not measured. {"M.E", "[M.E_Kid/M.Kid/Name = 5]"}, + {"M.E", "[Title = '[%DayLength%]']"}, } ctx := accessRuleCtx(t) for _, c := range cases { diff --git a/mdl/executor/validate_grant_refs.go b/mdl/executor/validate_grant_refs.go index 00d6ba3b6..76cc2c88e 100644 --- a/mdl/executor/validate_grant_refs.go +++ b/mdl/executor/validate_grant_refs.go @@ -100,6 +100,9 @@ type grantResolver struct { prog *ast.Program scriptEnums map[string][]string unjudgedEnums map[string]bool + // assocCatalog is the project's associations with their ends, for + // '[%CurrentUser%]' compared with an association; read on first use. + assocCatalog *associationCatalog } func (g *grantResolver) recordLater(stmt ast.Statement) {