caddyhttp: Escaping placeholders in CEL, add vars and vars_regexp (…

…caddyserver#6594)

* caddyhttp: Escaping placeholders in CEL

* Simplify some of the test cases

* Implement vars and vars_regexp in CEL

* dupl lint is dumb

* Better consts for the placeholder CEL shortcut

* Bump CEL version, register a few extensions

* Refactor s390x test script for readability

* Add retries for s390x to smooth over flakiness

* Switch to `ph` for the CEL shortcut (match it in templates cause why not)

.github/workflows/ci.yml

@@ -156,13 +156,35 @@ jobs:
           # short sha is enough?
           short_sha=$(git rev-parse --short HEAD)
 
+          # To shorten the following lines
+          ssh_opts="-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null"
+          ssh_host="[email protected]"
+
           # The environment is fresh, so there's no point in keeping accepting and adding the key.
-          rsync -arz -e "ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null" --progress --delete --exclude '.git' . "$CI_USER"@ci-s390x.caddyserver.com:/var/tmp/"$short_sha"
-          ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -t "$CI_USER"@ci-s390x.caddyserver.com "cd /var/tmp/$short_sha; go version; go env; printf "\n\n";CGO_ENABLED=0 go test -p 1 -tags nobadger -v ./..."
+          rsync -arz -e "ssh $ssh_opts" --progress --delete --exclude '.git' . "$ssh_host":/var/tmp/"$short_sha"
+          ssh $ssh_opts -t "$ssh_host" bash <<EOF
+          cd /var/tmp/$short_sha
+          go version
+          go env
+          printf "\n\n"
+          retries=3
+          exit_code=0
+          while ((retries > 0)); do
+            CGO_ENABLED=0 go test -p 1 -tags nobadger -v ./...
+            exit_code=$?
+            if ((exit_code == 0)); then
+              break
+            fi
+            echo "\n\nTest failed: \$exit_code, retrying..."
+            ((retries--))
+          done
+          echo "Remote exit code: \$exit_code"
+          exit \$exit_code
+          EOF
           test_result=$?
 
           # There's no need leaving the files around
-          ssh -o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null "$CI_USER"@ci-s390x.caddyserver.com "rm -rf /var/tmp/'$short_sha'"
+          ssh $ssh_opts "$ssh_host" "rm -rf /var/tmp/'$short_sha'"
 
           echo "Test exit code: $test_result"
           exit $test_result

.golangci.yml

@@ -171,6 +171,12 @@ issues:
     - path: modules/logging/filters.go
       linters:
         - dupl
+    - path: modules/caddyhttp/matchers.go
+      linters:
+        - dupl
+    - path: modules/caddyhttp/vars.go
+      linters:
+        - dupl
     - path: _test\.go
       linters:
         - errcheck

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/caddyserver/zerossl v0.1.3
 	github.com/dustin/go-humanize v1.0.1
 	github.com/go-chi/chi/v5 v5.0.12
-	github.com/google/cel-go v0.20.1
+	github.com/google/cel-go v0.21.0
 	github.com/google/uuid v1.6.0
 	github.com/klauspost/compress v1.17.8
 	github.com/klauspost/cpuid/v2 v2.2.7

go.sum

@@ -198,8 +198,8 @@ github.com/golang/snappy v0.0.4/go.mod h1:/XxbfmMg8lxefKM7IXC3fBNl/7bRcc72aCRzEW
 github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ=
 github.com/google/btree v1.1.2 h1:xf4v41cLI2Z6FxbKm+8Bu+m8ifhj15JuZ9sa0jZCMUU=
 github.com/google/btree v1.1.2/go.mod h1:qOPhT0dTNdNzV6Z/lhRX0YXUafgPLFUh+gZMl761Gm4=
-github.com/google/cel-go v0.20.1 h1:nDx9r8S3L4pE61eDdt8igGj8rf5kjYR3ILxWIpWNi84=
-github.com/google/cel-go v0.20.1/go.mod h1:kWcIzTsPX0zmQ+H3TirHstLLf9ep5QTsZBN9u4dOYLg=
+github.com/google/cel-go v0.21.0 h1:cl6uW/gxN+Hy50tNYvI691+sXxioCnstFzLp2WO4GCI=
+github.com/google/cel-go v0.21.0/go.mod h1:rHUlWCcBKgyEk+eV03RPdZUekPp6YcJwV0FxuUksYxc=
 github.com/google/certificate-transparency-go v1.0.21/go.mod h1:QeJfpSbVSfYc7RgB3gJFj9cbuQMMchQxrWXz8Ruopmg=
 github.com/google/certificate-transparency-go v1.1.8-0.20240110162603-74a5dd331745 h1:heyoXNxkRT155x4jTAiSv5BVSVkueifPUm+Q8LUXMRo=
 github.com/google/certificate-transparency-go v1.1.8-0.20240110162603-74a5dd331745/go.mod h1:zN0wUQgV9LjwLZeFHnrAbQi8hzMVvEWePyk+MhPOk7k=

modules/caddyhttp/celmatcher.go

@@ -126,6 +126,10 @@ func (m *MatchExpression) Provision(ctx caddy.Context) error {
 	// light (and possibly naïve) syntactic sugar
 	m.expandedExpr = placeholderRegexp.ReplaceAllString(m.Expr, placeholderExpansion)
 
+	// as a second pass, we'll strip the escape character from an escaped
+	// placeholder, so that it can be used as an input to other CEL functions
+	m.expandedExpr = escapedPlaceholderRegexp.ReplaceAllString(m.expandedExpr, escapedPlaceholderExpansion)
+
 	// our type adapter expands CEL's standard type support
 	m.ta = celTypeAdapter{}
 
@@ -159,14 +163,17 @@ func (m *MatchExpression) Provision(ctx caddy.Context) error {
 
 	// create the CEL environment
 	env, err := cel.NewEnv(
-		cel.Function(placeholderFuncName, cel.SingletonBinaryBinding(m.caddyPlaceholderFunc), cel.Overload(
-			placeholderFuncName+"_httpRequest_string",
+		cel.Function(CELPlaceholderFuncName, cel.SingletonBinaryBinding(m.caddyPlaceholderFunc), cel.Overload(
+			CELPlaceholderFuncName+"_httpRequest_string",
 			[]*cel.Type{httpRequestObjectType, cel.StringType},
 			cel.AnyType,
 		)),
-		cel.Variable("request", httpRequestObjectType),
+		cel.Variable(CELRequestVarName, httpRequestObjectType),
 		cel.CustomTypeAdapter(m.ta),
 		ext.Strings(),
+		ext.Bindings(),
+		ext.Lists(),
+		ext.Math(),
 		matcherLib,
 	)
 	if err != nil {
@@ -247,15 +254,15 @@ func (m MatchExpression) caddyPlaceholderFunc(lhs, rhs ref.Val) ref.Val {
 		return types.NewErr(
 			"invalid request of type '%v' to %s(request, placeholderVarName)",
 			lhs.Type(),
-			placeholderFuncName,
+			CELPlaceholderFuncName,
 		)
 	}
 	phStr, ok := rhs.(types.String)
 	if !ok {
 		return types.NewErr(
 			"invalid placeholder variable name of type '%v' to %s(request, placeholderVarName)",
 			rhs.Type(),
-			placeholderFuncName,
+			CELPlaceholderFuncName,
 		)
 	}
 
@@ -275,7 +282,7 @@ var httpRequestCELType = cel.ObjectType("http.Request", traits.ReceiverType)
 type celHTTPRequest struct{ *http.Request }
 
 func (cr celHTTPRequest) ResolveName(name string) (any, bool) {
-	if name == "request" {
+	if name == CELRequestVarName {
 		return cr, true
 	}
 	return nil, false
@@ -457,15 +464,15 @@ func CELMatcherDecorator(funcName string, fac CELMatcherFactory) interpreter.Int
 		callArgs := call.Args()
 		reqAttr, ok := callArgs[0].(interpreter.InterpretableAttribute)
 		if !ok {
-			return nil, errors.New("missing 'request' argument")
+			return nil, errors.New("missing 'req' argument")
 		}
 		nsAttr, ok := reqAttr.Attr().(interpreter.NamespacedAttribute)
 		if !ok {
-			return nil, errors.New("missing 'request' argument")
+			return nil, errors.New("missing 'req' argument")
 		}
 		varNames := nsAttr.CandidateVariableNames()
-		if len(varNames) != 1 || len(varNames) == 1 && varNames[0] != "request" {
-			return nil, errors.New("missing 'request' argument")
+		if len(varNames) != 1 || len(varNames) == 1 && varNames[0] != CELRequestVarName {
+			return nil, errors.New("missing 'req' argument")
 		}
 		matcherData, ok := callArgs[1].(interpreter.InterpretableConst)
 		if !ok {
@@ -524,7 +531,7 @@ func celMatcherStringListMacroExpander(funcName string) cel.MacroFactory {
 				return nil, eh.NewError(arg.ID(), "matcher arguments must be string constants")
 			}
 		}
-		return eh.NewCall(funcName, eh.NewIdent("request"), eh.NewList(matchArgs...)), nil
+		return eh.NewCall(funcName, eh.NewIdent(CELRequestVarName), eh.NewList(matchArgs...)), nil
 	}
 }
 
@@ -538,7 +545,7 @@ func celMatcherStringMacroExpander(funcName string) parser.MacroExpander {
 			return nil, eh.NewError(0, "matcher requires one argument")
 		}
 		if isCELStringExpr(args[0]) {
-			return eh.NewCall(funcName, eh.NewIdent("request"), args[0]), nil
+			return eh.NewCall(funcName, eh.NewIdent(CELRequestVarName), args[0]), nil
 		}
 		return nil, eh.NewError(args[0].ID(), "matcher argument must be a string literal")
 	}
@@ -572,7 +579,7 @@ func celMatcherJSONMacroExpander(funcName string) parser.MacroExpander {
 					return nil, eh.NewError(entry.AsMapEntry().Value().ID(), "matcher map values must be string or list literals")
 				}
 			}
-			return eh.NewCall(funcName, eh.NewIdent("request"), arg), nil
+			return eh.NewCall(funcName, eh.NewIdent(CELRequestVarName), arg), nil
 		case ast.UnspecifiedExprKind, ast.CallKind, ast.ComprehensionKind, ast.IdentKind, ast.ListKind, ast.LiteralKind, ast.SelectKind:
 			// appeasing the linter :)
 		}
@@ -646,7 +653,7 @@ func isCELCaddyPlaceholderCall(e ast.Expr) bool {
 	switch e.Kind() {
 	case ast.CallKind:
 		call := e.AsCall()
-		if call.FunctionName() == "caddyPlaceholder" {
+		if call.FunctionName() == CELPlaceholderFuncName {
 			return true
 		}
 	case ast.UnspecifiedExprKind, ast.ComprehensionKind, ast.IdentKind, ast.ListKind, ast.LiteralKind, ast.MapKind, ast.SelectKind, ast.StructKind:
@@ -701,16 +708,26 @@ func isCELStringListLiteral(e ast.Expr) bool {
 // expressions with a proper CEL function call; this is
 // just for syntactic sugar.
 var (
-	placeholderRegexp    = regexp.MustCompile(`{([a-zA-Z][\w.-]+)}`)
-	placeholderExpansion = `caddyPlaceholder(request, "${1}")`
+	// The placeholder may not be preceded by a backslash; the expansion
+	// will include the preceding character if it is not a backslash.
+	placeholderRegexp    = regexp.MustCompile(`([^\\]|^){([a-zA-Z][\w.-]+)}`)
+	placeholderExpansion = `${1}ph(req, "${2}")`
+
+	// As a second pass, we need to strip the escape character in front of
+	// the placeholder, if it exists.
+	escapedPlaceholderRegexp    = regexp.MustCompile(`\\{([a-zA-Z][\w.-]+)}`)
+	escapedPlaceholderExpansion = `{${1}}`
 
 	CELTypeJSON = cel.MapType(cel.StringType, cel.DynType)
 )
 
 var httpRequestObjectType = cel.ObjectType("http.Request")
 
 // The name of the CEL function which accesses Replacer values.
-const placeholderFuncName = "caddyPlaceholder"
+const CELPlaceholderFuncName = "ph"
+
+// The name of the CEL request variable.
+const CELRequestVarName = "req"
 
 const MatcherNameCtxKey = "matcher_name"