mirror of
https://github.com/instructkr/claw-code.git
synced 2026-06-05 17:32:43 -04:00
fix: expand ${VAR} and ~/ in MCP config fields (#92)
MCP server config now expands ${VAR} environment variable references
and ~/ home directory prefix in command, args, and url fields. Previously
these values were passed verbatim to execve/URL-parse, causing silent
"No such file or directory" failures for standard config patterns.
Generated with https://github.com/Yeachan-Heo/gajae-code
Co-authored-by: Gajae Code <dev@gajae-code.com>
This commit is contained in:
@@ -1923,7 +1923,7 @@ Original filing (2026-04-13): user requested a `-acp` parameter to support ACP p
|
|||||||
|
|
||||||
**Source.** Jobdori dogfood 2026-04-18 against `/tmp/cdC` on main HEAD `478ba55` in response to Clawhip pinpoint nudge at `1494714078965403848`. Second member of the "redaction-surface / reporting-surface is incomplete" sub-cluster after #90, and a direct sibling of #87 ("permission mode source invisible"): #87 is "fallback vs explicit" provenance loss; #91 is "alias vs canonical" provenance loss. Together with #87 they pin the permission-reporting surface from two angles. Different axis from the truth-audit cluster (#80–#86, #89): here the surface is not reporting a wrong value — it is canonicalizing an alias losslessly *and silently* in a way that loses the operator's intent.
|
**Source.** Jobdori dogfood 2026-04-18 against `/tmp/cdC` on main HEAD `478ba55` in response to Clawhip pinpoint nudge at `1494714078965403848`. Second member of the "redaction-surface / reporting-surface is incomplete" sub-cluster after #90, and a direct sibling of #87 ("permission mode source invisible"): #87 is "fallback vs explicit" provenance loss; #91 is "alias vs canonical" provenance loss. Together with #87 they pin the permission-reporting surface from two angles. Different axis from the truth-audit cluster (#80–#86, #89): here the surface is not reporting a wrong value — it is canonicalizing an alias losslessly *and silently* in a way that loses the operator's intent.
|
||||||
|
|
||||||
92. **MCP `command`, `args`, and `url` config fields are passed to `execve`/URL-parse **verbatim** — no `${VAR}` interpolation, no `~/` home expansion, no preflight check, no doctor warning — so standard config patterns silently fail at MCP connect time with confusing "No such file or directory" errors** — dogfooded 2026-04-18 on main HEAD `d0de86e` from `/tmp/cdE`. Every MCP stdio configuration on the web uses `${VAR}` / `~/...` syntax for command paths and credentials; `claw` stores them literally and hands the literal strings to `Command::new` at spawn time.
|
92. **DONE — MCP `command`, `args`, and `url` config fields are passed to `execve`/URL-parse **verbatim** — no `${VAR}` interpolation, no `~/` home expansion, no preflight check, no doctor warning — so standard config patterns silently fail at MCP connect time with confusing "No such file or directory" errors** — dogfooded 2026-04-18 on main HEAD `d0de86e` from `/tmp/cdE`. Every MCP stdio configuration on the web uses `${VAR}` / `~/...` syntax for command paths and credentials; `claw` stores them literally and hands the literal strings to `Command::new` at spawn time.
|
||||||
|
|
||||||
**Concrete repros.**
|
**Concrete repros.**
|
||||||
|
|
||||||
|
|||||||
@@ -2097,6 +2097,45 @@ fn parse_optional_oauth_config(
|
|||||||
}))
|
}))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// #92: expand `${VAR}` environment variable references and `~/` home directory
|
||||||
|
/// prefix in a config string value. Returns the expanded string.
|
||||||
|
fn expand_config_value(value: &str) -> String {
|
||||||
|
// Expand ${VAR} and $VAR references from the environment
|
||||||
|
let mut result = String::with_capacity(value.len());
|
||||||
|
let mut chars = value.chars().peekable();
|
||||||
|
while let Some(c) = chars.next() {
|
||||||
|
if c == '$' {
|
||||||
|
if chars.peek() == Some(&'{') {
|
||||||
|
// ${VAR} form
|
||||||
|
chars.next(); // consume '{'
|
||||||
|
let mut var_name = String::new();
|
||||||
|
for ch in chars.by_ref() {
|
||||||
|
if ch == '}' {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
var_name.push(ch);
|
||||||
|
}
|
||||||
|
if let Ok(val) = std::env::var(&var_name) {
|
||||||
|
result.push_str(&val);
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
// Bare $ — pass through
|
||||||
|
result.push(c);
|
||||||
|
}
|
||||||
|
} else if c == '~' && result.is_empty() {
|
||||||
|
// ~/... home directory expansion
|
||||||
|
if let Ok(home) = std::env::var("HOME") {
|
||||||
|
result.push_str(&home);
|
||||||
|
} else {
|
||||||
|
result.push(c);
|
||||||
|
}
|
||||||
|
} else {
|
||||||
|
result.push(c);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
fn parse_mcp_server_config(
|
fn parse_mcp_server_config(
|
||||||
server_name: &str,
|
server_name: &str,
|
||||||
value: &JsonValue,
|
value: &JsonValue,
|
||||||
@@ -2106,9 +2145,14 @@ fn parse_mcp_server_config(
|
|||||||
let server_type =
|
let server_type =
|
||||||
optional_string(object, "type", context)?.unwrap_or_else(|| infer_mcp_server_type(object));
|
optional_string(object, "type", context)?.unwrap_or_else(|| infer_mcp_server_type(object));
|
||||||
match server_type {
|
match server_type {
|
||||||
|
// #92: expand ${VAR} and ~/ in command, args, and url fields
|
||||||
"stdio" => Ok(McpServerConfig::Stdio(McpStdioServerConfig {
|
"stdio" => Ok(McpServerConfig::Stdio(McpStdioServerConfig {
|
||||||
command: expect_non_empty_string(object, "command", context)?.to_string(),
|
command: expand_config_value(expect_non_empty_string(object, "command", context)?),
|
||||||
args: optional_string_array(object, "args", context)?.unwrap_or_default(),
|
args: optional_string_array(object, "args", context)?
|
||||||
|
.unwrap_or_default()
|
||||||
|
.iter()
|
||||||
|
.map(|a| expand_config_value(a))
|
||||||
|
.collect(),
|
||||||
env: optional_string_map(object, "env", context)?.unwrap_or_default(),
|
env: optional_string_map(object, "env", context)?.unwrap_or_default(),
|
||||||
tool_call_timeout_ms: optional_u64(object, "toolCallTimeoutMs", context)?,
|
tool_call_timeout_ms: optional_u64(object, "toolCallTimeoutMs", context)?,
|
||||||
})),
|
})),
|
||||||
@@ -2119,7 +2163,8 @@ fn parse_mcp_server_config(
|
|||||||
object, context,
|
object, context,
|
||||||
)?)),
|
)?)),
|
||||||
"ws" => Ok(McpServerConfig::Ws(McpWebSocketServerConfig {
|
"ws" => Ok(McpServerConfig::Ws(McpWebSocketServerConfig {
|
||||||
url: expect_string(object, "url", context)?.to_string(),
|
// #92: expand ${VAR} and ~/ in URL
|
||||||
|
url: expand_config_value(expect_string(object, "url", context)?),
|
||||||
headers: optional_string_map(object, "headers", context)?.unwrap_or_default(),
|
headers: optional_string_map(object, "headers", context)?.unwrap_or_default(),
|
||||||
headers_helper: optional_string(object, "headersHelper", context)?.map(str::to_string),
|
headers_helper: optional_string(object, "headersHelper", context)?.map(str::to_string),
|
||||||
})),
|
})),
|
||||||
@@ -2127,7 +2172,8 @@ fn parse_mcp_server_config(
|
|||||||
name: expect_string(object, "name", context)?.to_string(),
|
name: expect_string(object, "name", context)?.to_string(),
|
||||||
})),
|
})),
|
||||||
"claudeai-proxy" => Ok(McpServerConfig::ManagedProxy(McpManagedProxyServerConfig {
|
"claudeai-proxy" => Ok(McpServerConfig::ManagedProxy(McpManagedProxyServerConfig {
|
||||||
url: expect_string(object, "url", context)?.to_string(),
|
// #92: expand ${VAR} and ~/ in URL
|
||||||
|
url: expand_config_value(expect_string(object, "url", context)?),
|
||||||
id: expect_string(object, "id", context)?.to_string(),
|
id: expect_string(object, "id", context)?.to_string(),
|
||||||
})),
|
})),
|
||||||
other => Err(ConfigError::Parse(format!(
|
other => Err(ConfigError::Parse(format!(
|
||||||
@@ -2149,7 +2195,8 @@ fn parse_mcp_remote_server_config(
|
|||||||
context: &str,
|
context: &str,
|
||||||
) -> Result<McpRemoteServerConfig, ConfigError> {
|
) -> Result<McpRemoteServerConfig, ConfigError> {
|
||||||
Ok(McpRemoteServerConfig {
|
Ok(McpRemoteServerConfig {
|
||||||
url: expect_string(object, "url", context)?.to_string(),
|
// #92: expand ${VAR} and ~/ in URL
|
||||||
|
url: expand_config_value(expect_string(object, "url", context)?),
|
||||||
headers: optional_string_map(object, "headers", context)?.unwrap_or_default(),
|
headers: optional_string_map(object, "headers", context)?.unwrap_or_default(),
|
||||||
headers_helper: optional_string(object, "headersHelper", context)?.map(str::to_string),
|
headers_helper: optional_string(object, "headersHelper", context)?.map(str::to_string),
|
||||||
oauth: parse_optional_mcp_oauth_config(object, context)?,
|
oauth: parse_optional_mcp_oauth_config(object, context)?,
|
||||||
|
|||||||
Reference in New Issue
Block a user