docs: document lenient shell expansion and security model

Christian Rocha and Charm Crush created 1 week ago

Updates the README and a handful of struct docs to describe the new
behavior: missing variables expand to empty, ${VAR:?message} is the
way to require a value, empty header values are dropped, extra_body
is a plain JSON passthrough, and crush.json should be treated as
trusted code because any $(...) in it runs at load time with the
user's shell privileges.

Co-Authored-By: Charm Crush <[e-mail]>

Change summary

README.md                 | 31 +++++++++++++++++++++++++------
internal/config/config.go | 24 +++++++++++++++++++++---
2 files changed, 46 insertions(+), 9 deletions(-)

Detailed changes

README.md

@@ -297,12 +297,31 @@ types: `stdio` for command-line servers, `http` for HTTP endpoints, and `sse`
 for Server-Sent Events.
 
 Shell-style value expansion (`$VAR`, `${VAR:-default}`, `$(command)`, quoting,
-and nesting (works in `command`, `args`, `env`, `headers`, and `url`, so
-file-based secrets like work out of the box, so you can use values like
-"$TOKEN"` and `"$(cat /path/to/secret/token)"``. Expansion runs through Crush's
-embedded shell, so the same syntax works on all supported systems, including
-Windows. Unset variables are an error; use `${VAR:-fallback}` to opt in to
-a default.
+nesting) works in `command`, `args`, `env`, `headers`, and `url`, so
+file-based secrets work out of the box. You can use values like `"$TOKEN"`
+or `"$(cat /path/to/secret/token)"`. Expansion runs through Crush's embedded
+shell, so the same syntax works on every supported system, Windows included.
+
+Unset variables expand to the empty string by default, matching bash. For
+required credentials, use `${VAR:?message}` so an unset variable fails loudly
+at load time with `message` instead of silently resolving to empty:
+
+```json
+{ "api_key": "${CODEBERG_TOKEN:?set CODEBERG_TOKEN}" }
+```
+
+Headers (both MCP `headers` and provider `extra_headers`) whose value
+resolves to the empty string are dropped from the outgoing request rather
+than sent as `Header:`. That keeps optional env-gated headers like
+`"OpenAI-Organization": "$OPENAI_ORG_ID"` clean when the variable is unset.
+
+Provider `extra_body` is a non-expanding JSON passthrough; put env-driven
+values in `extra_headers` or the provider's `api_key` / `base_url`, all of
+which do expand.
+
+> **Security note:** `crush.json` is trusted code. Any `$(...)` in it runs at
+> load time with your shell's privileges, before the UI appears. Don't launch
+> Crush in a directory whose `crush.json` you haven't reviewed.
 
 ```json
 {

internal/config/config.go

@@ -108,9 +108,22 @@ type ProviderConfig struct {
 	// Custom system prompt prefix.
 	SystemPromptPrefix string `json:"system_prompt_prefix,omitempty" jsonschema:"description=Custom prefix to add to system prompts for this provider"`
 
-	// Extra headers to send with each request to the provider.
+	// Extra headers to send with each request to the provider. Values
+	// run through shell expansion at config-load time, so $VAR and
+	// $(cmd) work the same way they do in MCP headers. A header whose
+	// value resolves to the empty string (unset bare $VAR under
+	// lenient nounset, $(echo), or literal "") is omitted from the
+	// outgoing request rather than sent as "Header:". See PLAN.md
+	// Phase 2 design decision #18.
 	ExtraHeaders map[string]string `json:"extra_headers,omitempty" jsonschema:"description=Additional HTTP headers to send with requests"`
-	// Extra body
+	// ExtraBody is merged verbatim into OpenAI-compatible request
+	// bodies. String values are NOT shell-expanded: this is a plain
+	// JSON passthrough so that arbitrary provider-extension fields
+	// (numbers, nested objects, booleans) round-trip without a
+	// recursive walker guessing at intent. If you need an env-var-
+	// driven value at request time, put it in extra_headers, or in
+	// the provider's top-level api_key / base_url, all of which do
+	// expand. See PLAN.md Phase 2 design decision #16.
 	ExtraBody map[string]any `json:"extra_body,omitempty" jsonschema:"description=Additional fields to include in request bodies, only works with openai-compatible providers"`
 
 	ProviderOptions map[string]any `json:"provider_options,omitempty" jsonschema:"description=Additional provider-specific options for this provider"`
@@ -174,7 +187,12 @@ type MCPConfig struct {
 	DisabledTools []string          `json:"disabled_tools,omitempty" jsonschema:"description=List of tools from this MCP server to disable,example=get-library-doc"`
 	Timeout       int               `json:"timeout,omitempty" jsonschema:"description=Timeout in seconds for MCP server connections,default=15,example=30,example=60,example=120"`
 
-	// TODO: maybe make it possible to get the value from the env
+	// Headers are HTTP headers for HTTP/SSE MCP servers. Values run
+	// through shell expansion at MCP startup, so $VAR and $(cmd)
+	// work. A header whose value resolves to the empty string (unset
+	// bare $VAR under lenient nounset, $(echo), or literal "") is
+	// omitted from the outgoing request rather than sent as
+	// "Header:". See PLAN.md Phase 2 design decision #18.
 	Headers map[string]string `json:"headers,omitempty" jsonschema:"description=HTTP headers for HTTP/SSE MCP servers"`
 }