fix(TASK-046): honor the --mcp-list inspection contract without a vault

Pure --mcp-list never reads secrets (only --mcp-add does), yet the mcp
action path always ran strict Vault::init, so a virgin environment with
no password file exited 1 instead of printing the empty listing. Route
the mcp_inspect path through the established Vault::init_lenient
constructor; get/remove/add stay on the strict init.

Make tests/inspection_flags.rs hermetic against host state: every
spawned binary gets a fresh fake HOME (and USERPROFILE) so a real
~/.coyote_password can't mask virgin-environment regressions, scrubs
IS_SANDBOX/COYOTE_PROVIDER/COYOTE_PLATFORM outside the sandbox-mode
tests, and pins CWD so workspace mcp.json discovery can't leak in.
New tests pin both sides of the boundary: the lenient list path (empty
and configured, never bootstrapping a password file) and the strict
vault gate for get/remove/add.
This commit is contained in:
Dark-Alex-17 committed 2026-09-16 23:09:59 -06:00
1 parent 879cf8e097
commit c2e30c12a6
2 files changed
+195 -2

No files matched your search

+9 -1
View File
@@ -259,7 +259,15 @@ async fn main() -> Result<()> {
} else { } else {
AppConfig::from_config(cfg)? AppConfig::from_config(cfg)?
}; };
let vault = Vault::init(&app_config)?; // Listing servers reads no secrets, so an unconfigured vault (no
// Listing servers reads no secrets, so an unconfigured vault (no
// password file) must not block the readout; every other MCP flag
// (get/remove/add) stays on the strict init.
let vault = if mcp_inspect {
Vault::init_lenient(&app_config)
} else {
Vault::init(&app_config)?
};
mcp::manage::handle(&cli, &vault)?; mcp::manage::handle(&cli, &vault)?;
+186 -1
View File
@@ -9,6 +9,11 @@
//! agent_info_on_empty_config_dir_bootstraps_builtins. Flags that write, //! agent_info_on_empty_config_dir_bootstraps_builtins. Flags that write,
//! like --sync-models, are NOT inspection flags and must stay on the //! like --sync-models, are NOT inspection flags and must stay on the
//! bootstrap path — see sync_models_stays_on_bootstrap_path. //! bootstrap path — see sync_models_stays_on_bootstrap_path.
//!
//! Every test runs against a fresh fake HOME and (unless sandbox mode is the
//! subject under test) a scrubbed IS_SANDBOX/provider env, so host state —
//! a real ~/.coyote_password or a sandbox shell — cannot mask
//! virgin-environment regressions.
use std::env; use std::env;
use std::fs; use std::fs;
@@ -27,11 +32,27 @@ fn fresh_config_dir(label: &str) -> PathBuf {
} }
fn probe_inspection_flag(flag: &str, require_stdout: bool) { fn probe_inspection_flag(flag: &str, require_stdout: bool) {
let tmp_dir = fresh_config_dir(flag.trim_start_matches("--")); let label = flag.trim_start_matches("--");
let tmp_dir = fresh_config_dir(label);
let home_dir = fresh_config_dir(&format!("{label}-home"));
let output = Command::new(env!("CARGO_BIN_EXE_coyote")) let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg(flag) .arg(flag)
.env("COYOTE_CONFIG_DIR", &tmp_dir) .env("COYOTE_CONFIG_DIR", &tmp_dir)
// A fresh fake HOME hides any real ~/.coyote_password on the host;
// USERPROFILE is the best-effort Windows equivalent (dirs resolves
// home via the Known Folder API there). HOME is set, never removed:
// dirs::home_dir() panics inside gman when home can't be determined.
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
// A leaked sandbox/provider env would reroute the vault and config
// paths under test.
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
// Workspace mcp.json discovery resolves from the CWD, so pin it to
// the isolated fake HOME to keep host state out of the probe.
.current_dir(&home_dir)
.stdin(Stdio::null()) .stdin(Stdio::null())
.output() .output()
.unwrap(); .unwrap();
@@ -40,7 +61,12 @@ fn probe_inspection_flag(flag: &str, require_stdout: bool) {
.unwrap() .unwrap()
.map(|entry| entry.unwrap().file_name()) .map(|entry| entry.unwrap().file_name())
.collect(); .collect();
assert!(
!home_dir.join(".coyote_password").exists(),
"{flag}: the lenient inspection path must never bootstrap a password file"
);
let _ = fs::remove_dir_all(&tmp_dir); let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!( assert!(
output.status.success(), output.status.success(),
@@ -90,10 +116,17 @@ fn mcp_list_on_empty_config_dir_writes_nothing() {
#[test] #[test]
fn agent_info_on_empty_config_dir_bootstraps_builtins() { fn agent_info_on_empty_config_dir_bootstraps_builtins() {
let tmp_dir = fresh_config_dir("agent-info"); let tmp_dir = fresh_config_dir("agent-info");
let home_dir = fresh_config_dir("agent-info-home");
let output = Command::new(env!("CARGO_BIN_EXE_coyote")) let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(["--agent", "adversary", "--info"]) .args(["--agent", "adversary", "--info"])
.env("COYOTE_CONFIG_DIR", &tmp_dir) .env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null()) .stdin(Stdio::null())
.output() .output()
.unwrap(); .unwrap();
@@ -102,6 +135,7 @@ fn agent_info_on_empty_config_dir_bootstraps_builtins() {
let builtins_installed = let builtins_installed =
agents_dir.is_dir() && fs::read_dir(&agents_dir).unwrap().next().is_some(); agents_dir.is_dir() && fs::read_dir(&agents_dir).unwrap().next().is_some();
let _ = fs::remove_dir_all(&tmp_dir); let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!( assert!(
output.status.success(), output.status.success(),
@@ -131,11 +165,15 @@ fn agent_info_on_empty_config_dir_bootstraps_builtins() {
#[test] #[test]
fn sandbox_list_secrets_prints_informational_message_and_writes_nothing() { fn sandbox_list_secrets_prints_informational_message_and_writes_nothing() {
let tmp_dir = fresh_config_dir("sandbox-list-secrets"); let tmp_dir = fresh_config_dir("sandbox-list-secrets");
let home_dir = fresh_config_dir("sandbox-list-secrets-home");
let output = Command::new(env!("CARGO_BIN_EXE_coyote")) let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg("--list-secrets") .arg("--list-secrets")
.env("COYOTE_CONFIG_DIR", &tmp_dir) .env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("IS_SANDBOX", "1") .env("IS_SANDBOX", "1")
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.current_dir(&home_dir)
.stdin(Stdio::null()) .stdin(Stdio::null())
.output() .output()
.unwrap(); .unwrap();
@@ -145,6 +183,7 @@ fn sandbox_list_secrets_prints_informational_message_and_writes_nothing() {
.map(|entry| entry.unwrap().file_name()) .map(|entry| entry.unwrap().file_name())
.collect(); .collect();
let _ = fs::remove_dir_all(&tmp_dir); let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!( assert!(
output.status.success(), output.status.success(),
@@ -175,6 +214,7 @@ fn sandbox_list_secrets_prints_informational_message_and_writes_nothing() {
#[test] #[test]
fn sandbox_mutating_vault_flag_stays_strict() { fn sandbox_mutating_vault_flag_stays_strict() {
let tmp_dir = fresh_config_dir("sandbox-delete-secret"); let tmp_dir = fresh_config_dir("sandbox-delete-secret");
let home_dir = fresh_config_dir("sandbox-delete-secret-home");
fs::write( fs::write(
tmp_dir.join("config.yaml"), tmp_dir.join("config.yaml"),
"model: dryrun:dry-model\n\ "model: dryrun:dry-model\n\
@@ -196,10 +236,14 @@ fn sandbox_mutating_vault_flag_stays_strict() {
.args(["--delete-secret", "probe"]) .args(["--delete-secret", "probe"])
.env("COYOTE_CONFIG_DIR", &tmp_dir) .env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("IS_SANDBOX", "1") .env("IS_SANDBOX", "1")
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.current_dir(&home_dir)
.stdin(Stdio::null()) .stdin(Stdio::null())
.output() .output()
.unwrap(); .unwrap();
let _ = fs::remove_dir_all(&tmp_dir); let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!( assert!(
!output.status.success(), !output.status.success(),
@@ -218,6 +262,142 @@ fn sandbox_mutating_vault_flag_stays_strict() {
); );
} }
// The --mcp-list leniency must not leak into the mutating MCP flags on the
// host path: with no password file (fresh fake HOME) and no sandbox env,
// --mcp-get/--mcp-remove/--mcp-add have to fail on the strict vault
// requirement rather than operate with an unconfigured vault.
fn probe_strict_mcp_flag(label: &str, args: &[&str]) {
let tmp_dir = fresh_config_dir(&format!("{label}-no-vault"));
let home_dir = fresh_config_dir(&format!("{label}-no-vault-home"));
fs::write(
tmp_dir.join("config.yaml"),
"model: dryrun:dry-model\n\
dry_run: true\n\
clients:\n\
\x20 - type: openai\n\
\x20 name: dryrun\n\
\x20 auth: none\n\
\x20 api_key: unused\n\
\x20 models:\n\
\x20 - name: dry-model\n\
\x20 max_input_tokens: 100000\n\
\x20 supports_function_calling: true\n\
save: false\n",
)
.unwrap();
fs::write(
tmp_dir.join("mcp.json"),
r#"{"mcpServers":{"probe-server":{"type":"stdio","command":"echo","args":["hi"]}}}"#,
)
.unwrap();
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.args(args)
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
// The strict path must not touch the server registry either.
let mcp_json_after = fs::read_to_string(tmp_dir.join("mcp.json")).unwrap();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
!output.status.success(),
"{label} without a vault: expected a non-zero exit, got {:?}\nstdout: {}",
output.status,
String::from_utf8_lossy(&output.stdout)
);
let combined = format!(
"{}{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
assert!(
combined.contains("A password file is required"),
"{label} without a vault: expected the strict vault error, got: {combined}"
);
assert!(
mcp_json_after.contains("probe-server"),
"{label} without a vault: mcp.json must be left untouched, got: {mcp_json_after}"
);
}
#[test]
fn mcp_mutating_flag_stays_strict_without_vault() {
probe_strict_mcp_flag("mcp-remove", &["--mcp-remove", "probe-server"]);
}
// --mcp-get is read-only but consumes resolved server config (which may
// embed secrets), so it stays on the strict vault path by design.
#[test]
fn mcp_get_stays_strict_without_vault() {
probe_strict_mcp_flag("mcp-get", &["--mcp-get", "probe-server"]);
}
#[test]
fn mcp_add_stays_strict_without_vault() {
probe_strict_mcp_flag("mcp-add", &["--mcp-add", "newsrv", "--", "echo", "hi"]);
}
// The listing leniency exercised with content: a configured, secret-free
// server must be listed even when no vault exists (no password file, no
// sandbox env), and the readout must not write anything new into the
// config dir.
#[test]
fn mcp_list_lists_configured_server_without_vault() {
let tmp_dir = fresh_config_dir("mcp-list-configured");
let home_dir = fresh_config_dir("mcp-list-configured-home");
fs::write(
tmp_dir.join("mcp.json"),
r#"{"mcpServers":{"probe-server":{"type":"stdio","command":"echo","args":["hi"]}}}"#,
)
.unwrap();
let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg("--mcp-list")
.env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null())
.output()
.unwrap();
let leftover: Vec<_> = fs::read_dir(&tmp_dir)
.unwrap()
.map(|entry| entry.unwrap().file_name())
.collect();
let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!(
output.status.success(),
"--mcp-list with a configured server and no vault: expected exit 0, got {:?}\nstderr: {}",
output.status,
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8_lossy(&output.stdout);
assert!(
stdout.contains("probe-server"),
"--mcp-list with a configured server: expected 'probe-server' in stdout, got: {stdout}"
);
assert_eq!(
leftover,
vec![std::ffi::OsString::from("mcp.json")],
"--mcp-list must not write into the config dir beyond the pre-seeded mcp.json"
);
}
// --sync-models downloads and writes models-override.yaml, so it is NOT an // --sync-models downloads and writes models-override.yaml, so it is NOT an
// inspection flag: on a pristine config dir it must take the bootstrap path // inspection flag: on a pristine config dir it must take the bootstrap path
// (builtins installed) and, off a terminal, fail on the missing config // (builtins installed) and, off a terminal, fail on the missing config
@@ -227,19 +407,24 @@ fn sandbox_mutating_vault_flag_stays_strict() {
#[test] #[test]
fn sync_models_stays_on_bootstrap_path() { fn sync_models_stays_on_bootstrap_path() {
let tmp_dir = fresh_config_dir("sync-models"); let tmp_dir = fresh_config_dir("sync-models");
let home_dir = fresh_config_dir("sync-models-home");
let output = Command::new(env!("CARGO_BIN_EXE_coyote")) let output = Command::new(env!("CARGO_BIN_EXE_coyote"))
.arg("--sync-models") .arg("--sync-models")
.env("COYOTE_CONFIG_DIR", &tmp_dir) .env("COYOTE_CONFIG_DIR", &tmp_dir)
.env("HOME", &home_dir)
.env("USERPROFILE", &home_dir)
.env_remove("IS_SANDBOX") .env_remove("IS_SANDBOX")
.env_remove("COYOTE_PROVIDER") .env_remove("COYOTE_PROVIDER")
.env_remove("COYOTE_PLATFORM") .env_remove("COYOTE_PLATFORM")
.current_dir(&home_dir)
.stdin(Stdio::null()) .stdin(Stdio::null())
.output() .output()
.unwrap(); .unwrap();
let bootstrapped = fs::read_dir(&tmp_dir).unwrap().next().is_some(); let bootstrapped = fs::read_dir(&tmp_dir).unwrap().next().is_some();
let _ = fs::remove_dir_all(&tmp_dir); let _ = fs::remove_dir_all(&tmp_dir);
let _ = fs::remove_dir_all(&home_dir);
assert!( assert!(
!output.status.success(), !output.status.success(),