fix(mcp-client): refuse a secret variable in mcp.json

plugins/rig-mcp-client/README.md

@@ -49,7 +49,7 @@ Servers are named in the `mcpServers` format other agents use, in two files:
 | Field | Meaning |
 | --- | --- |
 | `command`, `args` | A stdio server: the program rig starts, speaking MCP on its stdin and stdout, in a process group of its own that is killed when the server is stopped, removed or replaced, or rig exits (on Linux and macOS). Its stderr is dropped. A message of more than 64 MiB ends the connection, which says so. |
-| `env` | Variables added to its environment. The server gets rig's environment without rig's secrets (API keys); name a key here to give it one. |
+| `env` | Variables added to its environment. The server gets rig's environment except its `RIG_SECRET_*` and agent-only variables, so provider API keys from the shell reach it. |
 | `cwd` | Its working directory, relative to the project. |
 | `url`, `headers` | A streamable HTTP server, and headers sent with each request. |
 | `type` | Optional: `"stdio"`, or `"http"` (also `"streamable-http"`, `"streamableHttp"`). Without it, `url` means HTTP and `command` stdio. |
@@ -58,7 +58,7 @@ Servers are named in the `mcpServers` format other agents use, in two files:
 `${VAR}` in `command`, `args`, `env`, `cwd`, `url` and `headers` is the
 environment variable `VAR`, and `${VAR:-default}` is `default` when `VAR` is
 unset or empty. A server that uses an unset `${VAR}` with no default does not
-start, and says which field.
+start, and says which field. Nor does one that names a `${RIG_SECRET_*}`, in any letter case.
 
 A server's name is letters, digits, `_` and `-`, starting with a letter or
 `_` (as Gemini requires of tool names), without `__` and not ending in `_`, so

plugins/rig-mcp-client/src/config.rs

@@ -10,6 +10,7 @@ use std::path::{Path, PathBuf};
 use std::time::{SystemTime, UNIX_EPOCH};
 
 use rig_core::json_utils::remove_preserving_order;
+use rig_harness::harness_protocol::env::SECRET_PREFIX;
 use serde::{Deserialize, Serialize};
 use serde_json::ser::PrettyFormatter;
 use serde_json::{Map, Value};
@@ -207,7 +208,9 @@ fn refuse_fields(fields: &[(&str, bool)], whose: &str) -> Result<(), String> {
 
 /// `text` with each `${VAR}` replaced by the variable, and each
 /// `${VAR:-default}` by the variable unless it is unset or empty, else
-/// by `default`. Errors name `field`.
+/// by `default`. Errors name `field`; a secret variable
+/// ([`SECRET_PREFIX`], in any case, as Windows reads names) is refused,
+/// set or not.
 fn expand(text: &str, field: &str) -> Result<String, String> {
     let mut out = String::with_capacity(text.len());
     let mut rest = text;
@@ -222,6 +225,12 @@ fn expand(text: &str, field: &str) -> Result<String, String> {
             Some((name, default)) => (name, Some(default)),
             None => (inner, None),
         };
+        let prefix = name.get(..SECRET_PREFIX.len()).unwrap_or_default();
+        if prefix.eq_ignore_ascii_case(SECRET_PREFIX) {
+            return Err(format!(
+                "`{field}`: ${{{name}}} is a secret of rig's, which no server is given"
+            ));
+        }
         let value = std::env::var(name).ok();
         match (value, default) {
             (Some(value), Some(_)) if !value.is_empty() => out.push_str(&value),

plugins/rig-mcp-client/src/config/tests.rs

@@ -236,3 +236,27 @@ fn a_write_keeps_the_files_group() {
     assert_eq!(gid.ok(), Some(other));
     fs::remove_dir_all(dir).ok();
 }
+
+#[test]
+fn a_secret_variable_is_refused_by_name_and_never_expanded() {
+    let header = json!({ "url": "https://h", "headers": { "X": "${RIG_SECRET_CRYPT}" } });
+    let env = json!({ "command": "x", "env": { "K": "${RIG_SECRET_CRYPT:-none}" } });
+    let lower = json!({ "url": "https://h/${rig_secret_crypt:-none}" });
+    let refusal = |field: &str, name: &str| {
+        Err(format!(
+            "`{field}`: ${{{name}}} is a secret of rig's, which no server is given"
+        ))
+    };
+    assert_eq!(
+        ServerSpec::parse(&header),
+        refusal("headers.X", "RIG_SECRET_CRYPT")
+    );
+    assert_eq!(
+        ServerSpec::parse(&env),
+        refusal("env.K", "RIG_SECRET_CRYPT")
+    );
+    assert_eq!(
+        ServerSpec::parse(&lower),
+        refusal("url", "rig_secret_crypt")
+    );
+}