fix(cli): escape -p path so Windows backslashes survive CONNECT LOCAL (#644)

ako · claude · ako · commit 2e8ad253ea40 · 2026-06-06T10:33:58.000Z
The CLI fabricated `CONNECT LOCAL '&lt;path&gt;'` MDL from the -p flag value and
re-parsed it, so the lexer's unquoteString interpreted `\t`/`\n`/`\r` and
dropped escape backslashes -- e.g. `C:\temp\App.mpr` became `C:&lt;TAB&gt;emp\App.mpr`,
failing with "unable to open database file (14)". The positional .mdl argument
is a raw path, so it was unaffected; forward slashes worked as a workaround.

Add visitor.QuoteString (the exact inverse of unquoteString: `\`-&gt;`\\`,
`'`-&gt;`''`) and wrap the project path at every `CONNECT LOCAL '%s'` interpolation
site (check, describe, report, diff, query, lint, rename, exec, main.go). A raw
filesystem path must never be interpolated into MDL source unescaped.

Round-trip tests in visitor_connection_test.go cover \t/\n/\r paths, UNC paths,
apostrophes, and unix paths (unchanged). Symptom-table row added to fix-issue.md.

Co-Authored-By: Claude Opus 4.8 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md
@@ -52,6 +52,7 @@ to the symptom table below, so the next similar issue costs fewer reads.
 | `ALTER STYLING ON PAGE/SNIPPET ... SET ...` fails with `unsupported container type: PAGE` (and `DESCRIBE STYLING` silently shows "No widgets found"); even past that, design-property writes never reached builder-created pages | Two layered bugs: (1) the visitor emits uppercase `ContainerType` `"PAGE"`/`"SNIPPET"` but `execAlterStyling`/`execDescribeStyling` compared lowercase, so it fell through to the unsupported-container error; (2) ALTER STYLING used the reflection walker `walkPageWidgets` (legacy `ListPages`/`UpdatePage`), which can't locate widgets in MDL-builder pages and violates the mutator-pattern rule | `mdl/executor/cmd_styling.go` (`execAlterStyling`, `execDescribeStyling`) + `mdl/backend/mpr/page_mutator.go` | Normalise container type with `strings.ToLower`. Route ALTER STYLING through `ctx.Backend.OpenPageForMutation(unitID)` like ALTER PAGE; check `mutator.FindWidget`. Add `SetDesignProperty`/`RemoveDesignProperty`/`ClearDesignProperties` to the `PageMutator` interface, writing the widget's `Appearance.DesignProperties` BSON array (`Forms$DesignPropertyValue` → `Toggle`/`Option`/`Custom` value), preserving an existing custom kind on option updates. When an ALTER uses a container-type discriminator, mirror the casing fix already done for ALTER PAGE (#402). Issue #631 |
 | `declare $x list of T = empty;` (or any list-typed `declare`) passes `mxcli check` but Studio Pro rejects with CE0053 ("type not allowed") + CE0038 ("value required") | `declare` maps to a Create Variable activity, which Mendix forbids from producing a list — but the validator only flagged an empty list *used as a loop source* (MDL002), never the declaration itself | `mdl/executor/validate_microflow.go` → `walkBody` `*ast.DeclareStmt` case | Emit `MDL040` (SeverityError) for any `stmt.Type.Kind == ast.TypeListOf`, regardless of initializer. Lists must come from a microflow parameter, a `retrieve`, or `$x = create list of T;`. Also fix the synced skills that present declare-list as valid (`write-microflows.md`, `cheatsheet-variables.md`, `check-syntax.md`, `patterns-*`). Issue #607 |
 | `DESCRIBE PAGE` emits a widget whose name is a reserved keyword (`container List`, `dynamictext Template`) and the output fails `mxcli check` (`mismatched input 'List' expecting IDENTIFIER`); quoting it by hand (`container "List"`) also failed | The widget-name position `widgetV3` accepted only a bare `IDENTIFIER`, so neither the keyword nor a `QUOTED_IDENTIFIER` parsed; and DESCRIBE emitted the name unquoted. Note this is *not* the general reserved-word problem — qualified names / params / attributes already go through `identifierOrKeyword` (a 552-entry keyword allowlist). The gap is the ~103 strict-`IDENTIFIER` positions, of which widget names are the highest-impact | grammar `mdl/grammar/domains/MDLPage.g4` (`widgetV3`) + `mdl/visitor/visitor_page_v3.go` (`buildWidgetV3`) + `mdl/executor/cmd_pages_describe_output.go` | Widen the name position to `(IDENTIFIER \| QUOTED_IDENTIFIER)`, `make grammar`, and `unquoteIdentifier` it in the visitor. On output, quote via `executor.mdlIdent` (which lexes the name and quotes only when it does *not* lex as a bare `IDENTIFIER` — no hardcoded keyword list, no false positives like a widget named "Dot"). For other strict-`IDENTIFIER` name positions apply the same pattern: `pageParameter`/`snippetParameter` bare names were widened the same way (issue #114 — note the SHOW_PAGE colon arg it reported already worked via `identifierOrKeyword`, and DESCRIBE emits params `$`-prefixed so output was already safe; the fix only closed the bare-declaration gap). Still strict and unfixed: security `read(...)`/`write(...)` member lists. Issues #619, #114 |
+| `mxcli <cmd> -p "C:\\path\\App.mpr"` (Windows backslash path) fails with `unable to open database file (14)` and the echoed path is mangled (`C:\temp` → `C:<TAB>emp`, backslashes dropped); forward slashes (`-p C:/path/App.mpr`) work | The CLI fabricates `CONNECT LOCAL '<path>'` MDL from the flag value and re-parses it, so the lexer's `unquoteString` interprets `\t`/`\n`/`\r` and escape backslashes in the path. The positional `.mdl` arg is a raw path, so it's unaffected | every `cmd/mxcli/*.go` `fmt.Sprintf("CONNECT LOCAL '%s'...", projectPath)` site + `mdl/visitor/visitor_helpers.go` | Never interpolate a raw filesystem path into MDL source. Wrap it in `visitor.QuoteString(projectPath)` (escapes `\`→`\\`, `'`→`''` — the exact inverse of `unquoteString`) at every CONNECT-string site. Round-trip test in `mdl/visitor/visitor_connection_test.go`. Issue #644 |
 
 ---
 
diff --git a/cmd/mxcli/cmd_check.go b/cmd/mxcli/cmd_check.go
@@ -166,7 +166,7 @@ Examples:
 			defer exec.Close()
 
 			// Connect to project
-			connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+			connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 			for _, stmt := range connectProg.Statements {
 				if err := exec.Execute(stmt); err != nil {
 					fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
diff --git a/cmd/mxcli/cmd_describe.go b/cmd/mxcli/cmd_describe.go
@@ -170,7 +170,7 @@ Example:
 		exec.SetQuiet(true) // suppress status messages for programmatic output
 
 		// Connect
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error: %v\n", err)
diff --git a/cmd/mxcli/cmd_diff.go b/cmd/mxcli/cmd_diff.go
@@ -72,7 +72,7 @@ Examples:
 		defer logger.Close()
 		defer exec.Close()
 
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
@@ -145,7 +145,7 @@ Examples:
 		defer logger.Close()
 		defer exec.Close()
 
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
diff --git a/cmd/mxcli/cmd_exec.go b/cmd/mxcli/cmd_exec.go
@@ -39,7 +39,7 @@ Example:
 
 		// Auto-connect if project specified
 		if projectPath != "" {
-			connectCmd := fmt.Sprintf("CONNECT LOCAL '%s';", projectPath)
+			connectCmd := fmt.Sprintf("CONNECT LOCAL '%s';", visitor.QuoteString(projectPath))
 			prog, _ := visitor.Build(connectCmd)
 			for _, stmt := range prog.Statements {
 				if err := exec.Execute(stmt); err != nil {
diff --git a/cmd/mxcli/cmd_lint.go b/cmd/mxcli/cmd_lint.go
@@ -95,7 +95,7 @@ Examples:
 		defer logger.Close()
 		defer exec.Close()
 
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
diff --git a/cmd/mxcli/cmd_query.go b/cmd/mxcli/cmd_query.go
@@ -242,7 +242,7 @@ Examples:
 		defer exec.Close()
 
 		// Connect to project
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
diff --git a/cmd/mxcli/cmd_rename.go b/cmd/mxcli/cmd_rename.go
@@ -83,7 +83,7 @@ Example:
 		defer exec.Close()
 
 		// Connect
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s' FOR WRITING", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s' FOR WRITING", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error: %v\n", err)
diff --git a/cmd/mxcli/cmd_report.go b/cmd/mxcli/cmd_report.go
@@ -52,7 +52,7 @@ Examples:
 		defer logger.Close()
 		defer exec.Close()
 
-		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", projectPath))
+		connectProg, _ := visitor.Build(fmt.Sprintf("CONNECT LOCAL '%s'", visitor.QuoteString(projectPath)))
 		for _, stmt := range connectProg.Statements {
 			if err := exec.Execute(stmt); err != nil {
 				fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
diff --git a/cmd/mxcli/main.go b/cmd/mxcli/main.go
@@ -129,7 +129,7 @@ Examples:
 
 			// Auto-connect if project specified
 			if projectPath != "" {
-				commands = fmt.Sprintf("CONNECT LOCAL '%s'; %s", projectPath, commands)
+				commands = fmt.Sprintf("CONNECT LOCAL '%s'; %s", visitor.QuoteString(projectPath), commands)
 			}
 
 			prog, errs := visitor.Build(commands)
@@ -158,7 +158,7 @@ Examples:
 
 			// Auto-connect if project specified
 			if projectPath != "" {
-				if err := r.ExecuteString(fmt.Sprintf("CONNECT LOCAL '%s';", projectPath)); err != nil {
+				if err := r.ExecuteString(fmt.Sprintf("CONNECT LOCAL '%s';", visitor.QuoteString(projectPath))); err != nil {
 					fmt.Fprintf(os.Stderr, "Error connecting: %v\n", err)
 				}
 			}
@@ -216,7 +216,7 @@ func executeMDL(projectPath, mdlCmd string) {
 	defer logger.Close()
 	defer exec.Close()
 
-	fullCmd := fmt.Sprintf("CONNECT LOCAL '%s'; %s", projectPath, mdlCmd)
+	fullCmd := fmt.Sprintf("CONNECT LOCAL '%s'; %s", visitor.QuoteString(projectPath), mdlCmd)
 	prog, errs := visitor.Build(fullCmd)
 	if len(errs) > 0 {
 		for _, err := range errs {
diff --git a/mdl/visitor/visitor_connection_test.go b/mdl/visitor/visitor_connection_test.go
@@ -26,6 +26,52 @@ func TestConnect_Local(t *testing.T) {
 	}
 }
 
+// TestConnect_WindowsPathRaw documents the bug from issue #644: interpolating a
+// raw Windows path into CONNECT LOCAL '<path>' mangles it, because the MDL lexer
+// interprets `\t`/`\n`/`\r` and drops escape backslashes.
+func TestConnect_WindowsPathRaw(t *testing.T) {
+	const winPath = `C:\temp\App.mpr`
+	prog, errs := Build("CONNECT LOCAL '" + winPath + "'")
+	if len(errs) > 0 {
+		t.Fatalf("parse errors: %v", errs)
+	}
+	stmt := prog.Statements[0].(*ast.ConnectStmt)
+	// The raw interpolation corrupts the path (\t -> tab). This is the bug.
+	if stmt.Path == winPath {
+		t.Fatalf("expected raw interpolation to corrupt the path, but it survived")
+	}
+}
+
+// TestQuoteString_RoundTrip is the fix for #644: QuoteString(path) embedded in a
+// CONNECT LOCAL literal must parse back to the original path unchanged.
+func TestQuoteString_RoundTrip(t *testing.T) {
+	paths := []string{
+		`C:\Users\me\app\App.mpr`,
+		`C:\temp\App.mpr`,    // \t
+		`C:\new\App.mpr`,     // \n
+		`C:\reports\App.mpr`, // \r
+		`D:\a\b\c.mpr`,
+		`\\server\share\App.mpr`, // UNC
+		`/home/user/app.mpr`,     // unix, unchanged
+		`C:\It's a\trap.mpr`,     // apostrophe + \t
+	}
+	for _, p := range paths {
+		// Unit level: QuoteString is the exact inverse of unquoteString.
+		if got := unquoteString("'" + QuoteString(p) + "'"); got != p {
+			t.Errorf("unquoteString(QuoteString(%q)) = %q, want %q", p, got, p)
+		}
+		// Integration level: through the real parser via CONNECT LOCAL.
+		prog, errs := Build("CONNECT LOCAL '" + QuoteString(p) + "'")
+		if len(errs) > 0 {
+			t.Errorf("parse errors for %q: %v", p, errs)
+			continue
+		}
+		if stmt, ok := prog.Statements[0].(*ast.ConnectStmt); !ok || stmt.Path != p {
+			t.Errorf("CONNECT LOCAL round-trip for %q: got %+v", p, prog.Statements[0])
+		}
+	}
+}
+
 func TestDisconnect(t *testing.T) {
 	input := `DISCONNECT;`
 	prog, errs := Build(input)
diff --git a/mdl/visitor/visitor_helpers.go b/mdl/visitor/visitor_helpers.go
@@ -428,6 +428,18 @@ func buildDeleteBehavior(ctx parser.IDeleteBehaviorContext) ast.DeleteBehavior {
 	return ast.DeleteKeepReferences
 }
 
+// QuoteString escapes s for safe embedding inside an MDL single-quoted string
+// literal. It is the inverse of unquoteString: a backslash becomes `\\` and an
+// apostrophe becomes `”`. Use it whenever a runtime value (e.g. a filesystem
+// path) is interpolated into MDL source such as `CONNECT LOCAL '<path>'`, so a
+// Windows path like `C:\temp\App.mpr` round-trips unchanged instead of having
+// `\t`/`\n`/`\r` interpreted as escapes. See issue #644.
+func QuoteString(s string) string {
+	s = strings.ReplaceAll(s, `\`, `\\`)
+	s = strings.ReplaceAll(s, `'`, `''`)
+	return s
+}
+
 func unquoteString(s string) string {
 	// Remove surrounding quotes and unescape
 	if len(s) >= 2 && s[0] == '\'' && s[len(s)-1] == '\'' {
