fix(#1661): expand env vars for non-Unix shells (PowerShell/cmd) (#1666)

yottahmd · web-flow · commit 99b4999e9722 · 2026-02-14T03:41:30.000+09:00
diff --git a/internal/cmn/eval/envscope.go b/internal/cmn/eval/envscope.go
@@ -152,16 +152,42 @@ func (e *EnvScope) ToMap() map[string]string {
 // expandWithLookup expands $VAR and ${VAR} using the provided lookup function.
 // Single-quoted variables ('$VAR' or '${VAR}') and unknown variables are preserved.
 func expandWithLookup(s string, lookup func(key string) (string, bool)) string {
-	return reVarSubstitution.ReplaceAllStringFunc(s, func(match string) string {
-		key, ok := extractVarKey(match)
-		if !ok {
-			return match // Single-quoted - preserve
+	matches := reVarSubstitution.FindAllStringSubmatchIndex(s, -1)
+	if len(matches) == 0 {
+		return s
+	}
+
+	var b strings.Builder
+	last := 0
+	for _, loc := range matches {
+		b.WriteString(s[last:loc[0]])
+		last = loc[1]
+
+		match := s[loc[0]:loc[1]]
+		if isSingleQuotedVar(s, loc[0], loc[1]) {
+			b.WriteString(match)
+			continue
 		}
+
+		var key string
+		if loc[2] >= 0 { // Group 1: ${...}
+			key = s[loc[2]:loc[3]]
+		} else if loc[4] >= 0 { // Group 2: $VAR
+			key = s[loc[4]:loc[5]]
+		} else {
+			// Neither group captured — preserve original text.
+			b.WriteString(match)
+			continue
+		}
+
 		if val, found := lookup(key); found {
-			return val
+			b.WriteString(val)
+			continue
 		}
-		return match // Not found - preserve original
-	})
+		b.WriteString(match)
+	}
+	b.WriteString(s[last:])
+	return b.String()
 }
 
 // Expand expands ${VAR} and $VAR in s using this scope.
diff --git a/internal/cmn/eval/envscope_test.go b/internal/cmn/eval/envscope_test.go
@@ -748,6 +748,64 @@ func TestExpandWithLookup_SingleQuoted(t *testing.T) {
 	assert.Equal(t, "'$FOO' stays", result)
 }
 
+func TestExpandWithLookup_QuoteAdjacentCases(t *testing.T) {
+	lookup := func(key string) (string, bool) {
+		switch key {
+		case "FOO":
+			return "bar", true
+		case "BAR":
+			return "baz", true
+		default:
+			return "", false
+		}
+	}
+
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "BracedVarFollowedBySingleQuote",
+			input: "${FOO}'",
+			want:  "bar'",
+		},
+		{
+			name:  "SimpleVarFollowedBySingleQuote",
+			input: "$FOO'",
+			want:  "bar'",
+		},
+		{
+			name:  "SingleQuotedBracedPreserved",
+			input: "'${FOO}'",
+			want:  "'${FOO}'",
+		},
+		{
+			name:  "SingleQuotedSimplePreserved",
+			input: "'$FOO'",
+			want:  "'$FOO'",
+		},
+		{
+			name:  "MissingBracedVarFollowedBySingleQuote",
+			input: "${MISSING}'",
+			want:  "${MISSING}'",
+		},
+		{
+			name:  "MultipleVarsWithQuoteAdjacency",
+			input: "${FOO}' + $BAR'",
+			want:  "bar' + baz'",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got := expandWithLookup(tt.input, lookup)
+			assert.Equal(t, tt.want, got)
+		})
+	}
+}
+
 func TestEnvScope_Debug_NoParent(t *testing.T) {
 	scope := NewEnvScope(nil, true)
 	debug := scope.Debug()
diff --git a/internal/cmn/eval/expand.go b/internal/cmn/eval/expand.go
@@ -112,7 +112,7 @@ func expandWithShellContext(ctx context.Context, input string, opts *Options) (s
 		match := input[loc[0]:loc[1]]
 
 		// Single-quoted: preserve as-is.
-		if match[0] == '\'' {
+		if isSingleQuotedVar(input, loc[0], loc[1]) {
 			b.WriteString(match)
 			continue
 		}
diff --git a/internal/cmn/eval/expand_test.go b/internal/cmn/eval/expand_test.go
@@ -181,6 +181,42 @@ func TestExpandWithShellContext(t *testing.T) {
 		require.NoError(t, err)
 		assert.Equal(t, "'${VAR}'", result)
 	})
+
+	t.Run("VarFollowedBySingleQuote", func(t *testing.T) {
+		opts := NewOptions()
+		opts.Variables = []map[string]string{{"VAR": "value"}}
+
+		result, err := expandWithShellContext(context.Background(), "${VAR}'", opts)
+		require.NoError(t, err)
+		assert.Equal(t, "value'", result)
+	})
+
+	t.Run("SimpleVarFollowedBySingleQuote", func(t *testing.T) {
+		opts := NewOptions()
+		opts.Variables = []map[string]string{{"VAR": "value"}}
+
+		result, err := expandWithShellContext(context.Background(), "$VAR'", opts)
+		require.NoError(t, err)
+		assert.Equal(t, "value'", result)
+	})
+
+	t.Run("MissingVarFollowedBySingleQuoteWithoutExpandOS", func(t *testing.T) {
+		opts := NewOptions()
+		opts.ExpandOS = false
+
+		result, err := expandWithShellContext(context.Background(), "${MISSING}'", opts)
+		require.NoError(t, err)
+		assert.Equal(t, "${MISSING}'", result)
+	})
+
+	t.Run("SingleQuotedSimplePreserved", func(t *testing.T) {
+		opts := NewOptions()
+		opts.Variables = []map[string]string{{"VAR": "value"}}
+
+		result, err := expandWithShellContext(context.Background(), "'$VAR'", opts)
+		require.NoError(t, err)
+		assert.Equal(t, "'$VAR'", result)
+	})
 }
 
 func TestShellEnviron(t *testing.T) {
diff --git a/internal/cmn/eval/pipeline_test.go b/internal/cmn/eval/pipeline_test.go
@@ -800,6 +800,53 @@ func TestObject_ExplicitOSImportStillWorks(t *testing.T) {
 	assert.Equal(t, "echo /home/testuser", result.Command, "Explicitly imported OS var should be expanded")
 }
 
+func TestString_CommandLikeStringWithSingleQuoteAfterVar(t *testing.T) {
+	t.Parallel()
+
+	scope := NewEnvScope(nil, false).WithEntry("MY_VALUE", "hello", EnvSourceDAGEnv)
+
+	tests := []struct {
+		name  string
+		input string
+		want  string
+	}{
+		{
+			name:  "BracedVar",
+			input: `nu -c "print $'got: ${MY_VALUE}'"`,
+			want:  `nu -c "print $'got: hello'"`,
+		},
+		{
+			name:  "SimpleVar",
+			input: `nu -c "print $'got: $MY_VALUE'"`,
+			want:  `nu -c "print $'got: hello'"`,
+		},
+		{
+			name:  "MultipleVars",
+			input: `nu -c "print $'bucket: ${BUCKET_PREFIX}${PROJECT_BUCKET}'"`,
+			want:  `nu -c "print $'bucket: gs://my-bucket'"`,
+		},
+		{
+			name:  "MissingVarPreserved",
+			input: `nu -c "print $'got: ${MISSING}'"`,
+			want:  `nu -c "print $'got: ${MISSING}'"`,
+		},
+	}
+
+	scope = scope.
+		WithEntry("BUCKET_PREFIX", "gs://", EnvSourceDAGEnv).
+		WithEntry("PROJECT_BUCKET", "my-bucket", EnvSourceDAGEnv)
+	ctx := WithEnvScope(context.Background(), scope)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := String(ctx, tt.input, WithoutExpandEnv(), WithoutDollarEscape())
+			require.NoError(t, err)
+			require.Equal(t, tt.want, got)
+		})
+	}
+}
+
 func TestString_MultipleVariablesWithStepMapOnLast(t *testing.T) {
 	ctx := context.Background()
 
diff --git a/internal/cmn/eval/resolve.go b/internal/cmn/eval/resolve.go
@@ -7,8 +7,9 @@ import (
 	"strings"
 )
 
-// reVarSubstitution matches $VAR, ${VAR}, '$VAR', '${VAR}' patterns for variable substitution.
-var reVarSubstitution = regexp.MustCompile(`[']{0,1}\$\{([^}]+)\}[']{0,1}|[']{0,1}\$([a-zA-Z0-9_][a-zA-Z0-9_]*)[']{0,1}`)
+// reVarSubstitution matches ${...} and $VAR patterns for variable substitution.
+// Quote handling is done by callers based on surrounding characters.
+var reVarSubstitution = regexp.MustCompile(`\$\{([^}]+)\}|\$([a-zA-Z0-9_][a-zA-Z0-9_]*)`)
 
 // reQuotedJSONRef matches quoted JSON references like "${FOO.bar}" and simple variables like "${VAR}"
 var reQuotedJSONRef = regexp.MustCompile(`"\$\{([A-Za-z0-9_]\w*(?:\.[^}]+)?)\}"`)
@@ -110,34 +111,60 @@ func (r *resolver) resolveJSONSource(name string) (string, bool) {
 	return r.lookupScopeNonOS(name)
 }
 
-// extractVarKey extracts the variable key from a regex match.
-// Returns the key and false if the match is single-quoted.
-func extractVarKey(match string) (string, bool) {
-	if match[0] == '\'' && match[len(match)-1] == '\'' {
-		return "", false
-	}
-	if strings.HasPrefix(match, "${") {
-		return match[2 : len(match)-1], true
-	}
-	return match[1:], true
+// isSingleQuotedVar reports whether the matched variable token is enclosed
+// in single quotes in the original input (e.g., '${VAR}' or '$VAR').
+// Note: this is a simple adjacent-character heuristic. It may misidentify
+// nested quote contexts such as 'foo'${BAR}'baz' where the quote before
+// ${BAR} actually closes a prior segment. This is acceptable for the
+// targeted use cases (nu shell $'...' syntax, simple quoting).
+func isSingleQuotedVar(input string, start, end int) bool {
+	return start > 0 && end < len(input) && input[start-1] == '\'' && input[end] == '\''
 }
 
 // replaceVars substitutes $VAR and ${VAR} patterns using all resolver sources.
 // JSON path references (containing dots) are skipped; those are handled by expandReferences.
 func (r *resolver) replaceVars(template string) string {
-	return reVarSubstitution.ReplaceAllStringFunc(template, func(match string) string {
-		key, ok := extractVarKey(match)
-		if !ok {
-			return match
+	matches := reVarSubstitution.FindAllStringSubmatchIndex(template, -1)
+	if len(matches) == 0 {
+		return template
+	}
+
+	var b strings.Builder
+	last := 0
+	for _, loc := range matches {
+		b.WriteString(template[last:loc[0]])
+		last = loc[1]
+
+		match := template[loc[0]:loc[1]]
+		if isSingleQuotedVar(template, loc[0], loc[1]) {
+			b.WriteString(match)
+			continue
+		}
+
+		var key string
+		if loc[2] >= 0 { // Group 1: ${...}
+			key = template[loc[2]:loc[3]]
+		} else if loc[4] >= 0 { // Group 2: $VAR
+			key = template[loc[4]:loc[5]]
+		} else {
+			// Neither group captured — preserve original text.
+			b.WriteString(match)
+			continue
 		}
+
 		if strings.Contains(key, ".") {
-			return match
+			b.WriteString(match)
+			continue
 		}
 		if val, found := r.resolve(key); found {
-			return val
+			b.WriteString(val)
+			continue
 		}
-		return match
-	})
+		b.WriteString(match)
+	}
+
+	b.WriteString(template[last:])
+	return b.String()
 }
 
 // expandReferences resolves JSON path and step property references in the input.
diff --git a/internal/cmn/eval/resolve_test.go b/internal/cmn/eval/resolve_test.go
@@ -294,6 +294,18 @@ func TestReplaceVars_EdgeCases(t *testing.T) {
 			vars:     map[string]string{"FOO123": "value"},
 			want:     "value",
 		},
+		{
+			name:     "VarFollowedBySingleQuote",
+			template: "${FOO}'",
+			vars:     map[string]string{"FOO": "bar"},
+			want:     "bar'",
+		},
+		{
+			name:     "SimpleVarFollowedBySingleQuote",
+			template: "$FOO'",
+			vars:     map[string]string{"FOO": "bar"},
+			want:     "bar'",
+		},
 	}
 
 	for _, tt := range tests {
@@ -394,62 +406,6 @@ func TestReplaceVarsWithScope(t *testing.T) {
 	}
 }
 
-// --- extractVarKey ---
-
-func TestExtractVarKey(t *testing.T) {
-	tests := []struct {
-		name   string
-		match  string
-		wantK  string
-		wantOK bool
-	}{
-		{
-			name:   "BracedVar",
-			match:  "${FOO}",
-			wantK:  "FOO",
-			wantOK: true,
-		},
-		{
-			name:   "SimpleDollarVar",
-			match:  "$FOO",
-			wantK:  "FOO",
-			wantOK: true,
-		},
-		{
-			name:   "SingleQuotedBraced",
-			match:  "'${FOO}'",
-			wantK:  "",
-			wantOK: false,
-		},
-		{
-			name:   "SingleQuotedSimple",
-			match:  "'$FOO'",
-			wantK:  "",
-			wantOK: false,
-		},
-		{
-			name:   "VarWithUnderscore",
-			match:  "${FOO_BAR}",
-			wantK:  "FOO_BAR",
-			wantOK: true,
-		},
-		{
-			name:   "VarWithNumbers",
-			match:  "$VAR123",
-			wantK:  "VAR123",
-			wantOK: true,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			gotK, gotOK := extractVarKey(tt.match)
-			require.Equal(t, tt.wantK, gotK)
-			require.Equal(t, tt.wantOK, gotOK)
-		})
-	}
-}
-
 // --- ExpandReferences ---
 
 func TestExpandReferences(t *testing.T) {
diff --git a/internal/runtime/builtin/command/command.go b/internal/runtime/builtin/command/command.go
diff --git a/internal/runtime/builtin/command/eval_options_test.go b/internal/runtime/builtin/command/eval_options_test.go

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,7 @@ func expandWithShellContext(ctx context.Context, input string, opts *Options) (s`
`112`	`112`	`match := input[loc[0]:loc[1]]`
`113`	`113`
`114`	`114`	`// Single-quoted: preserve as-is.`
`115`		`- if match[0] == '\'' {`
	`115`	`+ if isSingleQuotedVar(input, loc[0], loc[1]) {`
`116`	`116`	`b.WriteString(match)`
`117`	`117`	`continue`
`118`	`118`	`}`