git.iko.soy/crush
on main help sign in
refactor(config): resolve shell vars via shell.ExpandValue
5dc30cfChristian Rocha
internal/config/resolve.go+118−115
compare with main
unifiedsplit
@@ -3,13 +3,17 @@ package config
3import (
3import (
4 "context"
4 "context"
5 "fmt"
5 "fmt"
6− "strings"
7 "time"
6 "time"
8
7
9 "github.com/charmbracelet/crush/internal/env"
8 "github.com/charmbracelet/crush/internal/env"
10 "github.com/charmbracelet/crush/internal/shell"
9 "github.com/charmbracelet/crush/internal/shell"
11)
10)
12
11
12+// resolveTimeout bounds how long a single ResolveValue call may spend
13+// inside shell expansion (including any command substitution). Matches
14+// the timeout used by the previous hand-rolled parser.
15+const resolveTimeout = 5 * time.Minute
16+
13type VariableResolver interface {
17type VariableResolver interface {
14 ResolveValue(value string) (string, error)
18 ResolveValue(value string) (string, error)
15}
19}
@@ -28,141 +32,140 @@ func IdentityResolver() VariableResolver {
28 return identityResolver{}
32 return identityResolver{}
29}
33}
30
34
31−type Shell interface {
35+// Expander is the single-value shell expansion seam used by
36+// shellVariableResolver. Production wires it to shell.ExpandValue; tests
37+// can inject a fake via WithExpander.
32− Exec(ctx context.Context, command string) (stdout, stderr string, err error)
38+type Expander func(ctx context.Context, value string, env []string) (string, error)
39+
40+// ShellResolverOption customizes shell variable resolver construction.
41+type ShellResolverOption func(*shellVariableResolver)
42+
43+// WithExpander overrides the expansion function used by the resolver.
44+// Primarily intended for tests; production callers should not need this.
45+func WithExpander(e Expander) ShellResolverOption {
46+ return func(r *shellVariableResolver) {
47+ if e != nil {
48+ r.expand = e
49+ }
50+ }
33}
51}
34
52
35type shellVariableResolver struct {
53type shellVariableResolver struct {
36− shell Shell
37− env env.Env
54+ env env.Env
55+ expand Expander
38}
56}
39
57
58+// NewShellVariableResolver returns a VariableResolver that delegates to
59+// the embedded shell (the same interpreter used by the bash tool and
60+// hooks). Supported constructs match shell.ExpandValue: $VAR, ${VAR},
61+// ${VAR:-default}, $(command), quoting, and escapes. Unset variables are
62+// an error; use ${VAR:-} to opt in to an empty fallback.
40−func NewShellVariableResolver(env env.Env) VariableResolver {
63+func NewShellVariableResolver(e env.Env, opts ...ShellResolverOption) VariableResolver {
41− return &shellVariableResolver{
64+ r := &shellVariableResolver{
42− env: env,
65+ env: e,
43− shell: shell.NewShell(
66+ expand: shell.ExpandValue,
44− &shell.Options{
45− Env: env.Env(),
46− },
47− ),
48 }
67 }
68+ for _, opt := range opts {
69+ opt(r)
70+ }
71+ return r
49}
72}
50
73
51−// ResolveValue is a method for resolving values, such as environment variables.
52−// it will resolve shell-like variable substitution anywhere in the string, including:
74+// ResolveValue resolves shell-style substitution anywhere in the string:
75+//
53−// - $(command) for command substitution
76+// - $(command) for command substitution, with full quoting and nesting.
54−// - $VAR or ${VAR} for environment variables
77+// - $VAR and ${VAR} for environment variables.
78+// - ${VAR:-default} / ${VAR:+alt} / ${VAR:?msg} for defaulting.
79+//
80+// Unset variables are a hard error (nounset), mirroring the historical
81+// behaviour of this resolver: silently expanding an unset variable to the
82+// empty string is exactly how broken credentials reach MCP servers.
55func (r *shellVariableResolver) ResolveValue(value string) (string, error) {
83func (r *shellVariableResolver) ResolveValue(value string) (string, error) {
56− // Special case: lone $ is an error (backward compatibility)
84+ // Preserve the historical backward-compat contract: a lone "$" is a
85+ // malformed config value, not a legal literal. The underlying shell
86+ // parser would accept it as a literal; we reject it here so existing
87+ // configs that relied on this validation still fail early.
57 if value == "$" {
88 if value == "$" {
58 return "", fmt.Errorf("invalid value format: %s", value)
89 return "", fmt.Errorf("invalid value format: %s", value)
59 }
90 }
60
91
61− // If no $ found, return as-is
92+ ctx, cancel := context.WithTimeout(context.Background(), resolveTimeout)
62− if !strings.Contains(value, "$") {
93+ defer cancel()
63− return value, nil
64− }
65−
66− result := value
67
94
68− // Handle command substitution: $(command)
95+ out, err := r.expand(ctx, value, r.env.Env())
69− for {
96+ if err != nil {
70− start := strings.Index(result, "$(")
97+ return "", sanitizeResolveError(value, err)
71− if start == -1 {
98+ }
72− break
99+ return out, nil
73− }
100+}
74
101
75− // Find matching closing parenthesis
102+// maxResolveErrBytes bounds the size of the inner error message surfaced
76− depth := 0
103+// from a resolution failure. Defense-in-depth on top of shell.ExpandValue's
77− end := -1
104+// own stderr budget: a custom Expander injected via WithExpander, or any
78− for i := start + 2; i < len(result); i++ {
105+// future non-shell error path, must still produce a user-safe message.
79− if result[i] == '(' {
106+const maxResolveErrBytes = 512
80− depth++
107+
81− } else if result[i] == ')' {
108+// sanitizeResolveError wraps an expansion error with the user-written
82− if depth == 0 {
109+// template (the pre-expansion string — it is what they typed, safe to
83− end = i
110+// surface) and a bounded, scrubbed rendering of the inner error message.
84− break
111+// Contract:
85− }
112+//
86− depth--
113+// - Never includes the resolved (post-expansion) value. This helper
114+// only receives the template and err, so a successful expansion
115+// result cannot reach it.
116+// - May include the template verbatim.
117+// - Truncates the inner error's message to maxResolveErrBytes and
118+// replaces embedded NULs and other non-printables (except tab and
119+// newline) with '?'.
120+//
121+// The returned error still unwraps to the original for errors.Is/As so
122+// callers can inspect typed sentinels; only the rendered message is
123+// scrubbed.
124+func sanitizeResolveError(template string, err error) error {
125+ if err == nil {
126+ return nil
87− }
127+ }
128+ return &resolveError{
129+ template: template,
130+ msg: scrubErrorMessage(err.Error()),
131+ inner: err,
132+ }
88− }
133+}
89
134
90− if end == -1 {
135+// resolveError is the concrete type returned by sanitizeResolveError.
91− return "", fmt.Errorf("unmatched $( in value: %s", value)
136+// Its Error() method returns the template + scrubbed inner message;
137+// Unwrap exposes the original error so errors.Is/As continue to work.
138+type resolveError struct {
139+ template string
140+ msg string
141+ inner error
92− }
142+}
93
143
94− command := result[start+2 : end]
144+func (e *resolveError) Error() string {
95− ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
145+ return fmt.Sprintf("resolving %q: %s", e.template, e.msg)
146+}
96
147
97− stdout, _, err := r.shell.Exec(ctx, command)
148+func (e *resolveError) Unwrap() error { return e.inner }
98− cancel()
99− if err != nil {
100− return "", fmt.Errorf("command execution failed for '%s': %w", command, err)
101− }
102
149
103− // Replace the $(command) with the output
150+// scrubErrorMessage bounds the message to maxResolveErrBytes bytes and
104− replacement := strings.TrimSpace(stdout)
151+// replaces non-printable bytes (anything outside ASCII printable, tab, or
105− result = result[:start] + replacement + result[end+1:]
152+// newline) with '?'. Mirrors shell.sanitizeStderr but operates on a
153+// string rather than raw command stderr and runs at the config layer,
154+// so arbitrary Expander error text is also sanitized.
155+func scrubErrorMessage(s string) string {
156+ if len(s) > maxResolveErrBytes {
157+ s = s[:maxResolveErrBytes]
106 }
158 }
107−
159+ out := make([]byte, len(s))
108− // Handle environment variables: $VAR and ${VAR}
160+ for i := 0; i < len(s); i++ {
109− searchStart := 0
161+ c := s[i]
110− for {
162+ if c == '\t' || c == '\n' || (c >= 0x20 && c < 0x7f) {
111− start := strings.Index(result[searchStart:], "$")
163+ out[i] = c
112− if start == -1 {
113− break
114− }
115− start += searchStart // Adjust for the offset
116−
117− // Skip if this is part of $( which we already handled
118− if start+1 < len(result) && result[start+1] == '(' {
119− // Skip past this $(...)
120− searchStart = start + 1
121 continue
164 continue
122 }
165 }
123− var varName string
166+ out[i] = '?'
124− var end int
125−
126− if start+1 < len(result) && result[start+1] == '{' {
127− // Handle ${VAR} format
128− closeIdx := strings.Index(result[start+2:], "}")
129− if closeIdx == -1 {
130− return "", fmt.Errorf("unmatched ${ in value: %s", value)
131− }
132− varName = result[start+2 : start+2+closeIdx]
133− end = start + 2 + closeIdx + 1
134− } else {
135− // Handle $VAR format - variable names must start with letter or underscore
136− if start+1 >= len(result) {
137− return "", fmt.Errorf("incomplete variable reference at end of string: %s", value)
138− }
139−
140− if result[start+1] != '_' &&
141− (result[start+1] < 'a' || result[start+1] > 'z') &&
142− (result[start+1] < 'A' || result[start+1] > 'Z') {
143− return "", fmt.Errorf("invalid variable name starting with '%c' in: %s", result[start+1], value)
144− }
145−
146− end = start + 1
147− for end < len(result) && (result[end] == '_' ||
148− (result[end] >= 'a' && result[end] <= 'z') ||
149− (result[end] >= 'A' && result[end] <= 'Z') ||
150− (result[end] >= '0' && result[end] <= '9')) {
151− end++
152− }
153− varName = result[start+1 : end]
154− }
155−
156− envValue := r.env.Get(varName)
157− if envValue == "" {
158− return "", fmt.Errorf("environment variable %q not set", varName)
159− }
160−
161− result = result[:start] + envValue + result[end:]
162− searchStart = start + len(envValue) // Continue searching after the replacement
163 }
167 }
164−
165− return result, nil
168+ return string(out)
166}
169}
167
170
168type environmentVariableResolver struct {
171type environmentVariableResolver struct {
@@ -177,11 +180,11 @@ func NewEnvironmentVariableResolver(env env.Env) VariableResolver {
177
180
178// ResolveValue resolves environment variables from the provided env.Env.
181// ResolveValue resolves environment variables from the provided env.Env.
179func (r *environmentVariableResolver) ResolveValue(value string) (string, error) {
182func (r *environmentVariableResolver) ResolveValue(value string) (string, error) {
180− if !strings.HasPrefix(value, "$") {
183+ if len(value) == 0 || value[0] != '$' {
181 return value, nil
184 return value, nil
182 }
185 }
183
186
184− varName := strings.TrimPrefix(value, "$")
187+ varName := value[1:]
185 resolvedValue := r.env.Get(varName)
188 resolvedValue := r.env.Get(varName)
186 if resolvedValue == "" {
189 if resolvedValue == "" {
187 return "", fmt.Errorf("environment variable %q not set", varName)
190 return "", fmt.Errorf("environment variable %q not set", varName)
internal/config/resolve_test.go+190−229
@@ -3,260 +3,221 @@ package config
3import (
3import (
4 "context"
4 "context"
5 "errors"
5 "errors"
6+ "strings"
6 "testing"
7 "testing"
7
8
8 "github.com/charmbracelet/crush/internal/env"
9 "github.com/charmbracelet/crush/internal/env"
9 "github.com/stretchr/testify/require"
10 "github.com/stretchr/testify/require"
10)
11)
11
12
12−// mockShell implements the Shell interface for testing
13+// fakeExpander returns a canned value/error for the last passed value and
14+// records the context, raw value, and env slice it was called with. It
15+// lets the config-layer tests assert on delegation behaviour without
16+// spinning up a real interpreter — real-shell coverage lives in
17+// internal/shell/expand_test.go and resolve_real_test.go.
13−type mockShell struct {
18+type fakeExpander struct {
14− execFunc func(ctx context.Context, command string) (stdout, stderr string, err error)
19+ expand func(ctx context.Context, value string, env []string) (string, error)
20+ lastValue string
21+ lastEnv []string
22+ calls int
15}
23}
16
24
17−func (m *mockShell) Exec(ctx context.Context, command string) (stdout, stderr string, err error) {
25+func (f *fakeExpander) Expand(ctx context.Context, value string, env []string) (string, error) {
18− if m.execFunc != nil {
26+ f.calls++
19− return m.execFunc(ctx, command)
27+ f.lastValue = value
28+ f.lastEnv = env
29+ if f.expand == nil {
30+ return value, nil
20 }
31 }
21− return "", "", nil
32+ return f.expand(ctx, value, env)
22}
33}
23
34
24−func TestShellVariableResolver_ResolveValue(t *testing.T) {
35+func TestShellVariableResolver_DelegatesToExpander(t *testing.T) {
25− tests := []struct {
36+ t.Parallel()
26− name string
27− value string
28− envVars map[string]string
29− shellFunc func(ctx context.Context, command string) (stdout, stderr string, err error)
30− expected string
31− expectError bool
32− }{
33− {
34− name: "non-variable string returns as-is",
35− value: "plain-string",
36− expected: "plain-string",
37− },
38− {
39− name: "environment variable resolution",
40− value: "$HOME",
41− envVars: map[string]string{"HOME": "/home/user"},
42− expected: "/home/user",
43− },
44− {
45− name: "missing environment variable returns error",
46− value: "$MISSING_VAR",
47− envVars: map[string]string{},
48− expectError: true,
49− },
50
37
51− {
38+ fe := &fakeExpander{
52− name: "shell command with whitespace trimming",
53− value: "$(echo ' spaced ')",
54− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
39+ expand: func(_ context.Context, value string, _ []string) (string, error) {
55− if command == "echo ' spaced '" {
40+ if value == "hello $FOO" {
56− return " spaced \n", "", nil
41+ return "hello bar", nil
57− }
58− return "", "", errors.New("unexpected command")
59− },
60− expected: "spaced",
61− },
62− {
63− name: "shell command execution error",
64− value: "$(false)",
65− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
66− return "", "", errors.New("command failed")
67− },
68− expectError: true,
69− },
42+ }
70− {
71− name: "invalid format returns error",
72− value: "$",
43+ return value, nil
73− expectError: true,
74 },
44 },
75 }
45 }
76
46
77− for _, tt := range tests {
78− t.Run(tt.name, func(t *testing.T) {
79− testEnv := env.NewFromMap(tt.envVars)
47+ e := env.NewFromMap(map[string]string{"FOO": "bar"})
80− resolver := &shellVariableResolver{
48+ r := NewShellVariableResolver(e, WithExpander(fe.Expand))
81− shell: &mockShell{execFunc: tt.shellFunc},
82− env: testEnv,
83− }
84
49
85− result, err := resolver.ResolveValue(tt.value)
50+ got, err := r.ResolveValue("hello $FOO")
51+ require.NoError(t, err)
52+ require.Equal(t, "hello bar", got)
53+ require.Equal(t, 1, fe.calls)
54+ require.Equal(t, "hello $FOO", fe.lastValue)
55+ require.Contains(t, fe.lastEnv, "FOO=bar")
56+}
86
57
87− if tt.expectError {
58+func TestShellVariableResolver_LoneDollarIsError(t *testing.T) {
59+ t.Parallel()
60+
61+ // Lone "$" must short-circuit before reaching the expander: the
62+ // underlying shell parser would accept it as a literal, but this
63+ // resolver has historically rejected it and callers depend on
64+ // that early-fail behaviour.
65+ fe := &fakeExpander{}
66+ r := NewShellVariableResolver(env.NewFromMap(nil), WithExpander(fe.Expand))
67+
68+ _, err := r.ResolveValue("$")
88− require.Error(t, err)
69+ require.Error(t, err)
89− } else {
70+ require.Equal(t, 0, fe.calls, "expander must not be called for lone $")
90− require.NoError(t, err)
91− require.Equal(t, tt.expected, result)
92− }
93− })
94− }
95}
71}
96
72
97−func TestShellVariableResolver_EnhancedResolveValue(t *testing.T) {
73+func TestShellVariableResolver_PassesThroughLiterals(t *testing.T) {
98− tests := []struct {
74+ t.Parallel()
99− name string
75+
100− value string
76+ fe := &fakeExpander{
101− envVars map[string]string
77+ expand: func(_ context.Context, value string, _ []string) (string, error) {
102− shellFunc func(ctx context.Context, command string) (stdout, stderr string, err error)
103− expected string
104− expectError bool
105− }{
106− {
107− name: "command substitution within string",
108− value: "Bearer $(echo token123)",
109− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
110− if command == "echo token123" {
111− return "token123\n", "", nil
112− }
113− return "", "", errors.New("unexpected command")
114− },
115− expected: "Bearer token123",
116− },
117− {
118− name: "environment variable within string",
119− value: "Bearer $TOKEN",
120− envVars: map[string]string{"TOKEN": "sk-ant-123"},
121− expected: "Bearer sk-ant-123",
122− },
123− {
124− name: "environment variable with braces within string",
125− value: "Bearer ${TOKEN}",
126− envVars: map[string]string{"TOKEN": "sk-ant-456"},
127− expected: "Bearer sk-ant-456",
128− },
129− {
130− name: "mixed command and environment substitution",
131− value: "$USER-$(date +%Y)-$HOST",
132− envVars: map[string]string{
133− "USER": "testuser",
134− "HOST": "localhost",
135− },
136− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
137− if command == "date +%Y" {
138− return "2024\n", "", nil
139− }
140− return "", "", errors.New("unexpected command")
141− },
142− expected: "testuser-2024-localhost",
143− },
144− {
145− name: "multiple command substitutions",
146− value: "$(echo hello) $(echo world)",
147− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
148− switch command {
149− case "echo hello":
150− return "hello\n", "", nil
78+ return value, nil
151− case "echo world":
152− return "world\n", "", nil
153− }
154− return "", "", errors.New("unexpected command")
155− },
156− expected: "hello world",
157− },
158− {
159− name: "nested parentheses in command",
160− value: "$(echo $(echo inner))",
161− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
162− if command == "echo $(echo inner)" {
163− return "nested\n", "", nil
164− }
165− return "", "", errors.New("unexpected command")
166− },
167− expected: "nested",
168− },
169− {
170− name: "lone dollar with non-variable chars",
171− value: "prefix$123suffix", // Numbers can't start variable names
172− expectError: true,
173− },
174− {
175− name: "dollar with special chars",
176− value: "a$@b$#c", // Special chars aren't valid in variable names
177− expectError: true,
178− },
179− {
180− name: "empty environment variable substitution",
181− value: "Bearer $EMPTY_VAR",
182− envVars: map[string]string{},
183− expectError: true,
184− },
185− {
186− name: "unmatched command substitution opening",
187− value: "Bearer $(echo test",
188− expectError: true,
189− },
190− {
191− name: "unmatched environment variable braces",
192− value: "Bearer ${TOKEN",
193− expectError: true,
194− },
195− {
196− name: "command substitution with error",
197− value: "Bearer $(false)",
198− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
199− return "", "", errors.New("command failed")
200− },
201− expectError: true,
202− },
203− {
204− name: "complex real-world example",
205− value: "Bearer $(cat /tmp/token.txt | base64 -w 0)",
206− shellFunc: func(ctx context.Context, command string) (stdout, stderr string, err error) {
207− if command == "cat /tmp/token.txt | base64 -w 0" {
208− return "c2stYW50LXRlc3Q=\n", "", nil
209− }
210− return "", "", errors.New("unexpected command")
211− },
212− expected: "Bearer c2stYW50LXRlc3Q=",
213− },
214− {
215− name: "environment variable with underscores and numbers",
216− value: "Bearer $API_KEY_V2",
217− envVars: map[string]string{"API_KEY_V2": "sk-test-123"},
218− expected: "Bearer sk-test-123",
219− },
220− {
221− name: "no substitution needed",
222− value: "Bearer sk-ant-static-token",
223− expected: "Bearer sk-ant-static-token",
224− },
225− {
226− name: "incomplete variable at end",
227− value: "Bearer $",
228− expectError: true,
229− },
230− {
231− name: "variable with invalid character",
232− value: "Bearer $VAR-NAME", // Hyphen not allowed in variable names
233− expectError: true,
234− },
235− {
236− name: "multiple invalid variables",
237− value: "$1$2$3",
238− expectError: true,
239 },
79 },
240 }
80 }
81+ r := NewShellVariableResolver(env.NewFromMap(nil), WithExpander(fe.Expand))
241
82
242− for _, tt := range tests {
83+ got, err := r.ResolveValue("plain-string")
243− t.Run(tt.name, func(t *testing.T) {
84+ require.NoError(t, err)
244− testEnv := env.NewFromMap(tt.envVars)
85+ require.Equal(t, "plain-string", got)
245− resolver := &shellVariableResolver{
246− shell: &mockShell{execFunc: tt.shellFunc},
247− env: testEnv,
248− }
86+}
249
87
250− result, err := resolver.ResolveValue(tt.value)
88+func TestShellVariableResolver_WrapsErrorsWithTemplate(t *testing.T) {
89+ t.Parallel()
251
90
252− if tt.expectError {
91+ inner := errors.New("cat: /run/secrets/x: permission denied")
253− require.Error(t, err)
92+ fe := &fakeExpander{
254− } else {
93+ expand: func(_ context.Context, _ string, _ []string) (string, error) {
255− require.NoError(t, err)
94+ return "", inner
256− require.Equal(t, tt.expected, result)
257− }
258− })
95+ },
259 }
96 }
97+ r := NewShellVariableResolver(env.NewFromMap(nil), WithExpander(fe.Expand))
98+
99+ _, err := r.ResolveValue("$(cat /run/secrets/x)")
100+ require.Error(t, err)
101+ require.ErrorIs(t, err, inner)
102+ require.Contains(t, err.Error(), "$(cat /run/secrets/x)")
103+ require.Contains(t, err.Error(), "permission denied")
104+}
105+
106+func TestSanitizeResolveError(t *testing.T) {
107+ t.Parallel()
108+
109+ t.Run("nil passes through", func(t *testing.T) {
110+ t.Parallel()
111+ require.NoError(t, sanitizeResolveError("anything", nil))
112+ })
113+
114+ t.Run("includes template and wraps inner", func(t *testing.T) {
115+ t.Parallel()
116+ inner := errors.New("cat: /run/secrets/x: permission denied")
117+ got := sanitizeResolveError("$(cat /run/secrets/x)", inner)
118+ require.Error(t, got)
119+ require.ErrorIs(t, got, inner)
120+ require.Contains(t, got.Error(), "$(cat /run/secrets/x)")
121+ require.Contains(t, got.Error(), "permission denied")
122+ })
123+
124+ t.Run("unwrap preserves original for errors.Is", func(t *testing.T) {
125+ t.Parallel()
126+ inner := errors.New("sentinel")
127+ got := sanitizeResolveError("$FOO", inner)
128+ require.ErrorIs(t, got, inner)
129+ })
130+
131+ t.Run("truncates over-budget inner message", func(t *testing.T) {
132+ t.Parallel()
133+ // Inner message holds far more than the budget. After
134+ // sanitization the rendered inner portion must not exceed
135+ // maxResolveErrBytes, and the characters beyond the budget
136+ // (marked by a distinct tail sentinel) must be gone.
137+ const tailSentinel = "TAIL_SENTINEL_BEYOND_BUDGET"
138+ body := strings.Repeat("x", maxResolveErrBytes)
139+ inner := errors.New(body + tailSentinel)
140+
141+ got := sanitizeResolveError("$TEMPLATE", inner)
142+ require.Error(t, got)
143+
144+ prefix := `resolving "$TEMPLATE": `
145+ rendered := got.Error()
146+ require.True(
147+ t,
148+ strings.HasPrefix(rendered, prefix),
149+ "rendered error must start with template prefix",
150+ )
151+ innerRendered := strings.TrimPrefix(rendered, prefix)
152+ require.LessOrEqual(
153+ t,
154+ len(innerRendered),
155+ maxResolveErrBytes,
156+ "inner message must be bounded to maxResolveErrBytes",
157+ )
158+ require.NotContains(
159+ t,
160+ rendered,
161+ tailSentinel,
162+ "content past the budget must not leak",
163+ )
164+ })
165+
166+ t.Run("replaces non-printable bytes", func(t *testing.T) {
167+ t.Parallel()
168+ // NUL, BEL, ESC, DEL, and a UTF-8 high byte should all be
169+ // scrubbed to '?'. Tab and newline are preserved because
170+ // they show up legitimately in command stderr.
171+ inner := errors.New("ok\x00bad\x07\x1b\x7f\xffend\ttab\nline")
172+ got := sanitizeResolveError("$T", inner)
173+ rendered := got.Error()
174+
175+ require.NotContains(t, rendered, "\x00")
176+ require.NotContains(t, rendered, "\x07")
177+ require.NotContains(t, rendered, "\x1b")
178+ require.NotContains(t, rendered, "\x7f")
179+ require.NotContains(t, rendered, "\xff")
180+ require.Contains(t, rendered, "ok?bad????end\ttab\nline")
181+ })
182+
183+ t.Run("scrubbing does not depend on shell.ExpandValue upstream", func(t *testing.T) {
184+ t.Parallel()
185+ // A custom Expander can inject arbitrary error text. The
186+ // config-layer helper is the single chokepoint; it must
187+ // bound + scrub regardless of the error source.
188+ nasty := strings.Repeat("A", maxResolveErrBytes+64) + "\x00BEYOND"
189+ fe := &fakeExpander{
190+ expand: func(_ context.Context, _ string, _ []string) (string, error) {
191+ return "", errors.New(nasty)
192+ },
193+ }
194+ r := NewShellVariableResolver(env.NewFromMap(nil), WithExpander(fe.Expand))
195+
196+ _, err := r.ResolveValue("$T")
197+ require.Error(t, err)
198+ require.NotContains(t, err.Error(), "BEYOND", "over-budget tail must not leak")
199+ require.NotContains(t, err.Error(), "\x00", "non-printables must be scrubbed")
200+ })
201+}
202+
203+func TestScrubErrorMessage(t *testing.T) {
204+ t.Parallel()
205+
206+ t.Run("bounds output to maxResolveErrBytes", func(t *testing.T) {
207+ t.Parallel()
208+ got := scrubErrorMessage(strings.Repeat("a", maxResolveErrBytes*3))
209+ require.Len(t, got, maxResolveErrBytes)
210+ })
211+
212+ t.Run("preserves printable ASCII tab and newline", func(t *testing.T) {
213+ t.Parallel()
214+ require.Equal(t, "a\tb\nc d!", scrubErrorMessage("a\tb\nc d!"))
215+ })
216+
217+ t.Run("replaces control and non-ASCII bytes", func(t *testing.T) {
218+ t.Parallel()
219+ require.Equal(t, "a?b??c", scrubErrorMessage("a\x01b\x1b\xe2c"))
220+ })
260}
221}
261
222
262func TestEnvironmentVariableResolver_ResolveValue(t *testing.T) {
223func TestEnvironmentVariableResolver_ResolveValue(t *testing.T) {