fix: regex substitution for $ signs in upstream path handling before running envsubst (#2524)

* Perform a regex replace of $NUM to $$NUM before running envsubst

* Perform a regex replace of $NUM to $$NUM before running envsubst

* add test case; fix linter warnings; add method documentation

Signed-off-by: Jan Larwig <jan@larwig.com>

* add changelog entry

Signed-off-by: Jan Larwig <jan@larwig.com>

---------

Signed-off-by: Jan Larwig <jan@larwig.com>
Co-authored-by: Jan Larwig <jan@larwig.com>

CHANGELOG.md

@@ -12,6 +12,7 @@
 - [#2605](https://github.com/oauth2-proxy/oauth2-proxy/pull/2605) fix: show login page on broken cookie (@Primexz)
 - [#2743](https://github.com/oauth2-proxy/oauth2-proxy/pull/2743) feat: allow use more possible google admin-sdk api scopes (@BobDu)
 - [#2359](https://github.com/oauth2-proxy/oauth2-proxy/pull/2359) feat: add SourceHut (sr.ht) provider(@bitfehler)
+-[#2524](https://github.com/oauth2-proxy/oauth2-proxy/pull/2524) fix: regex substitution for $ signs in upstream path handling before running envsubst (@dashkan / @tuunit)
 
 # V7.10.0
 

pkg/apis/options/load.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"os"
 	"reflect"
+	"regexp"
 	"strings"
 
 	"github.com/a8m/envsubst"
@@ -155,7 +156,8 @@ func LoadYAML(configFileName string, into interface{}) error {
 	return nil
 }
 
-// Performs the heavy lifting of the LoadYaml function
+// loadAndParseYaml reads the config from the filesystem and
+// execute the environment variable substitution
 func loadAndParseYaml(configFileName string) ([]byte, error) {
 	if configFileName == "" {
 		return nil, errors.New("no configuration file provided")
@@ -166,12 +168,26 @@ func loadAndParseYaml(configFileName string) ([]byte, error) {
 		return nil, fmt.Errorf("unable to load config file: %w", err)
 	}
 
-	// We now parse over the yaml with env substring, and fill in the ENV's
-	buffer, err := envsubst.Bytes(unparsedBuffer)
+	modifiedBuffer, err := normalizeSubstitution(unparsedBuffer)
+	if err != nil {
+		return nil, fmt.Errorf("error normalizing substitution string : %w", err)
+	}
+
+	buffer, err := envsubst.Bytes(modifiedBuffer)
 	if err != nil {
 		return nil, fmt.Errorf("error in substituting env variables : %w", err)
 	}
 
 	return buffer, nil
-
+}
+
+// normalizeSubstitution normalizes dollar signs ($) with numerals like
+// $1 or $2 properly by correctly escaping them
+func normalizeSubstitution(unparsedBuffer []byte) ([]byte, error) {
+	unparsedString := string(unparsedBuffer)
+
+	regexPattern := regexp.MustCompile(`\$(\d+)`)
+
+	substitutedString := regexPattern.ReplaceAllString(unparsedString, `$$$$1`)
+	return []byte(substitutedString), nil
 }

pkg/apis/options/load_test.go

@@ -487,6 +487,31 @@ sub:
 					StringOption: "Bob",
 				},
 			}),
+			Entry("with a config file containing $ signs for things other than environment variables", loadYAMLTableInput{
+				configFile: []byte(`
+stringOption: /$1
+stringSliceOption:
+- /$1
+- ^/(.*)$
+- api/$1
+- api/(.*)$
+- ^/api/(.*)$
+- /api/$1`),
+				input: &TestOptions{},
+				expectedOutput: &TestOptions{
+					StringOption: "/$1",
+					TestOptionSubStruct: TestOptionSubStruct{
+						StringSliceOption: []string{
+							"/$1",
+							"^/(.*)$",
+							"api/$1",
+							"api/(.*)$",
+							"^/api/(.*)$",
+							"/api/$1",
+						},
+					},
+				},
+			}),
 		)
 	})