From c5fbe4a0ea94d945399fb0b6d7dbceb32d7a1369 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 6 Jan 2026 23:58:40 +0000 Subject: [PATCH] feat: Add safe environment variable expansion Co-authored-by: dylan --- pkg/common/envexpand.go | 140 +++++++++++++++++++++++++++++++++++ pkg/common/envexpand_test.go | 113 ++++++++++++++++++++++++++++ 2 files changed, 253 insertions(+) create mode 100644 pkg/common/envexpand.go create mode 100644 pkg/common/envexpand_test.go diff --git a/pkg/common/envexpand.go b/pkg/common/envexpand.go new file mode 100644 index 000000000..68c383615 --- /dev/null +++ b/pkg/common/envexpand.go @@ -0,0 +1,140 @@ +package common + +import ( + "os" +) + +// ExpandEnvSafe expands environment variables in s, while intentionally avoiding +// shell-special expansions that are common in regex patterns and YAML comments. +// +// Supported expansions: +// - $VAR +// - ${VAR} +// +// Where VAR matches: [A-Za-z_][A-Za-z0-9_]* +// +// Notably, it does NOT expand: +// - shell special parameters like $? / $@ / $1 +// - incomplete brace forms like "${VAR" (missing closing brace) +// - non-identifier forms like "$(cmd)" +// +// If '$' is preceded by an odd number of backslashes (e.g. "\$FOO"), it is +// treated as escaped and will not be expanded. +func ExpandEnvSafe(s string) string { + return expandEnvSafe(s, os.Getenv) +} + +func expandEnvSafe(s string, getenv func(string) string) string { + if s == "" { + return s + } + + // Build lazily to avoid allocations when no expansions happen. + out := make([]byte, 0, len(s)) + changed := false + + for i := 0; i < len(s); i++ { + if s[i] != '$' { + out = append(out, s[i]) + continue + } + + // If '$' is escaped (odd number of backslashes immediately preceding), + // keep it literal. + if isEscapedDollar(s, i) { + out = append(out, '$') + continue + } + + // Need at least one character after '$' to expand. + if i+1 >= len(s) { + out = append(out, '$') + continue + } + + // ${VAR} form (only if there is a closing brace). + if s[i+1] == '{' { + end := indexByteFrom(s, '}', i+2) + if end == -1 { + // Incomplete; treat as literal. + out = append(out, '$') + continue + } + name := s[i+2 : end] + if isEnvVarName(name) { + out = append(out, getenv(name)...) + changed = true + i = end + continue + } + + // Not a valid env var name; keep literal. + out = append(out, s[i:end+1]...) + i = end + continue + } + + // $VAR form. + if isEnvVarNameStart(s[i+1]) { + j := i + 2 + for j < len(s) && isEnvVarNameChar(s[j]) { + j++ + } + name := s[i+1 : j] + out = append(out, getenv(name)...) + changed = true + i = j - 1 + continue + } + + // Otherwise treat '$' literally (covers $?, $@, $1, $(, $$, etc). + out = append(out, '$') + } + + if !changed { + return s + } + return string(out) +} + +func indexByteFrom(s string, b byte, start int) int { + for i := start; i < len(s); i++ { + if s[i] == b { + return i + } + } + return -1 +} + +func isEnvVarNameStart(b byte) bool { + return (b >= 'A' && b <= 'Z') || (b >= 'a' && b <= 'z') || b == '_' +} + +func isEnvVarNameChar(b byte) bool { + return isEnvVarNameStart(b) || (b >= '0' && b <= '9') +} + +func isEnvVarName(name string) bool { + if name == "" { + return false + } + if !isEnvVarNameStart(name[0]) { + return false + } + for i := 1; i < len(name); i++ { + if !isEnvVarNameChar(name[i]) { + return false + } + } + return true +} + +func isEscapedDollar(s string, dollarIdx int) bool { + // Count consecutive backslashes immediately before '$'. + n := 0 + for i := dollarIdx - 1; i >= 0 && s[i] == '\\'; i-- { + n++ + } + return n%2 == 1 +} + diff --git a/pkg/common/envexpand_test.go b/pkg/common/envexpand_test.go new file mode 100644 index 000000000..1e22da054 --- /dev/null +++ b/pkg/common/envexpand_test.go @@ -0,0 +1,113 @@ +package common + +import ( + "os" + "testing" +) + +func TestExpandEnvSafe(t *testing.T) { + t.Setenv("FOO", "BAR") + + tests := []struct { + name string + in string + want string + }{ + { + name: "expand $VAR", + in: "x$FOO y", + want: "xBAR y", + }, + { + name: "expand ${VAR}", + in: "x${FOO}y", + want: "xBARy", + }, + { + name: "do not expand $?", + in: "$?", + want: "$?", + }, + { + name: "do not expand $@", + in: "$@", + want: "$@", + }, + { + name: "do not expand $1", + in: "$1", + want: "$1", + }, + { + name: "do not mangle incomplete ${VAR", + in: "${SOMETHING", + want: "${SOMETHING", + }, + { + name: "do not expand $(...)", + in: "$(", + want: "$(", + }, + { + name: "do not expand invalid brace name", + in: "${1}", + want: "${1}", + }, + { + name: "escaped dollar does not expand", + in: `\$FOO`, + want: `\$FOO`, + }, + { + name: "regex example preserves $?, $@, and ${ without closing brace", + in: `detectors: + - name: Custom Detector + regex: + secret: |- + (?i)password=([^ ]+) + exclude_regexes_capture: + - |- + \\$?\\([A-Z$@0-9] + - |- # ${SOMETHING + ^foo$`, + want: `detectors: + - name: Custom Detector + regex: + secret: |- + (?i)password=([^ ]+) + exclude_regexes_capture: + - |- + \\$?\\([A-Z$@0-9] + - |- # ${SOMETHING + ^foo$`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ExpandEnvSafe(tt.in) + if got != tt.want { + t.Fatalf("ExpandEnvSafe mismatch:\n in: %q\n got: %q\n want: %q", tt.in, got, tt.want) + } + }) + } +} + +func TestExpandEnvSafe_DoesNotMatchOsExpandEnvForShellSpecials(t *testing.T) { + // This test documents the enterprise failure mode: os.ExpandEnv removes + // shell-special parameters like $? and $@, which can appear in regexes. + // + // If Go changes os.ExpandEnv behavior in the future, this test can be + // adjusted/removed; ExpandEnvSafe must continue to preserve them. + in := `\\$?\\([A-Z$@0-9]` + + got := ExpandEnvSafe(in) + if got != in { + t.Fatalf("expected ExpandEnvSafe to preserve input:\n got: %q\n in: %q", got, in) + } + + if os.ExpandEnv(in) == in { + t.Fatalf("expected os.ExpandEnv to differ from input for this case; update this test if behavior changes") + } +} +