Skip to content

Commit c350740

Browse files
committed
fix(security): add $$ escape for literal dollar sign in config var expansion (M6)
os.Expand has no escape mechanism — $$50 is not a literal $50, it's treated as variable expansion for "$" (which is empty), producing "50". Any config value containing a literal $ (API keys, passwords, system prompts) would be silently corrupted. Fix: replace os.Expand with a custom expandEnv that: - Supports $$ as an escape for a single literal $ - Parses ${VAR} and $VAR shell-style variable references - Falls through bare $ as a literal (no crash on trailing $) The new implementation is a drop-in replacement with identical behavior for normal ${VAR} and $VAR patterns, plus the $$ escape. Fixes M6 from security audit.
1 parent 9eb1c77 commit c350740

1 file changed

Lines changed: 71 additions & 2 deletions

File tree

internal/config/loader.go

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@
88
//
99
// Both config files are optional. Missing files are silently ignored.
1010
// String values in config files support ${VAR} environment variable
11-
// substitution (e.g. "api_key": "${MY_API_KEY}").
11+
// substitution (e.g. "api_key": "${MY_API_KEY}"). Use $$ for a literal
12+
// dollar sign.
1213
package config
1314

1415
import (
@@ -366,8 +367,76 @@ func loadFile(path string) FileConfig {
366367
}
367368

368369
// expandEnv replaces ${VAR} or $VAR with environment variable values.
370+
// Supports $$ as an escape for a literal dollar sign.
369371
func expandEnv(s string) string {
370-
return os.Expand(s, os.Getenv)
372+
var buf strings.Builder
373+
i := 0
374+
for j := 0; j < len(s); j++ {
375+
if s[j] != '$' {
376+
continue
377+
}
378+
buf.WriteString(s[i:j])
379+
380+
// $$ → literal $
381+
if j+1 < len(s) && s[j+1] == '$' {
382+
buf.WriteByte('$')
383+
i = j + 2
384+
j++ // skip second $
385+
continue
386+
}
387+
388+
// Find variable name: ${VAR} or $VAR or $VAR_NAME
389+
name, w := parseVarName(s[j+1:])
390+
i = j + 1 + w
391+
392+
if name == "" {
393+
// $ followed by non-identifier: emit as-is
394+
buf.WriteByte('$')
395+
continue
396+
}
397+
buf.WriteString(os.Getenv(name))
398+
}
399+
buf.WriteString(s[i:])
400+
return buf.String()
401+
}
402+
403+
// parseVarName extracts a shell variable name from s, which is the part
404+
// after the $ sign. Returns (name, width) where width is how many bytes
405+
// the variable reference consumed (including braces for ${VAR}).
406+
// Returns ("", 0) for no match (bare $) or ("", 1) for $?/$!/etc.
407+
func parseVarName(s string) (string, int) {
408+
if len(s) == 0 {
409+
return "", 0
410+
}
411+
if s[0] == '{' {
412+
// ${VAR}
413+
for k := 1; k < len(s); k++ {
414+
if s[k] == '}' {
415+
return s[1:k], k + 1
416+
}
417+
}
418+
return "", len(s) // unterminated — consume everything
419+
}
420+
// $VAR or $VAR_NAME123
421+
if !isVarStart(s[0]) {
422+
return "", 1 // $@, $*, $#, $?, $-, $$, $!, $0...
423+
}
424+
// Parse the rest of the name
425+
k := 1
426+
for k < len(s) && isVarCont(s[k]) {
427+
k++
428+
}
429+
return s[:k], k
430+
}
431+
432+
// isVarStart returns true for characters that can start a variable name.
433+
func isVarStart(c byte) bool {
434+
return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || c == '_'
435+
}
436+
437+
// isVarCont returns true for characters that can continue a variable name.
438+
func isVarCont(c byte) bool {
439+
return isVarStart(c) || (c >= '0' && c <= '9')
371440
}
372441

373442
// ── Environment Variable Loading ───────────────────────────────────────

0 commit comments

Comments
 (0)