Merge pull request #1167 from alliasgher/fix-envsubst-strict-transforms

envsubst: enforce strict mode for transformation operators

Author: stefanprodan

envsubst/eval_test.go

@@ -292,6 +292,28 @@ func TestExpandStrict(t *testing.T) {
 			output:  "default",
 			wantErr: nil,
 		},
+		// missing with a transformation operator should still error.
+		// Regression coverage for fluxcd/flux2#5836.
+		{params: map[string]string{}, input: "${#missing}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing^}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing^^}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing,}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing,,}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing:0}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing:0:10}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing/pattern/replacement}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing//pattern/replacement}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing/#pattern/replacement}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing/%pattern/replacement}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing#pattern}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing##pattern}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing%pattern}", wantErr: errVarNotSet},
+		{params: map[string]string{}, input: "${missing%%pattern}", wantErr: errVarNotSet},
+		// Default-providing operators must still succeed when the var is
+		// missing — exclusion must not regress these cases.
+		{params: map[string]string{}, input: "${missing:-fallback}", output: "fallback"},
+		{params: map[string]string{}, input: "${missing:=assigned}", output: "assigned"},
+		{params: map[string]string{}, input: "${missing=assigned}", output: "assigned"},
 	}
 
 	for _, expr := range expressions {

envsubst/template.go

@@ -131,7 +131,12 @@ func (t *Template) evalFunc(s *state, node *parse.FuncNode) error {
 
 	v, exists := s.mapper(node.Param)
 
-	if node.Name == "" && !exists {
+	// A variable is missing in strict mode (mapper returns exists=false) when
+	// it is referenced bare (${var}) or with a transformation operator that
+	// reads the value (e.g. ${#var}, ${var^^}, ${var/foo/bar}). Default-
+	// providing operators (-, :-, +, :+, =, :=, ?, :?) intentionally handle
+	// the unset case, so leave them alone.
+	if !exists && !isDefaultOp(node.Name) {
 		return fmt.Errorf("%w: %q", errVarNotSet, node.Param)
 	}
 	fn := lookupFunc(node.Name, len(args))
@@ -140,6 +145,20 @@ func (t *Template) evalFunc(s *state, node *parse.FuncNode) error {
 	return err
 }
 
+// isDefaultOp reports whether name is a POSIX-style parameter expansion
+// operator that tolerates an unset variable and supplies its own fallback
+// value (e.g. ${var:-default}, ${var:+set}, ${var:=assign}, ${var:?err}).
+// These operators are excluded from the strict-mode "variable not set"
+// check because the unset case is part of their contract.
+func isDefaultOp(name string) bool {
+	switch name {
+	case "-", "+", "=", "?",
+		":-", ":+", ":=", ":?":
+		return true
+	}
+	return false
+}
+
 // lookupFunc returns the parameters substitution function by name. If the
 // named function does not exists, a default function is returned.
 func lookupFunc(name string, args int) substituteFunc {