diff --git a/src/config/paths.rs b/src/config/paths.rs index 268e497..177864d 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -445,7 +445,7 @@ pub(crate) fn is_rag_sidecar_name(name: &str) -> bool { /// /// Callers must run this BEFORE unlinking the primary `.yaml`. If the YAML goes first /// and this then fails, the RAG disappears from `list_rags()` — so the user can no -/// longer select it to retry — while its `allowedDomains` entry keeps being injected +/// longer select it to retry — while its network allow entry keeps being injected /// into every sandbox launch. pub(crate) fn remove_rag_sidecars(dir: &Path, name: &str) -> Result<()> { let duckdb_path = dir.join(format!("{name}.duckdb")); diff --git a/src/rag/mod.rs b/src/rag/mod.rs index abe5319..29f298a 100644 --- a/src/rag/mod.rs +++ b/src/rag/mod.rs @@ -16,6 +16,7 @@ use self::provider::RagProvider; // `create()`, so `providers` itself must stay in scope — do not collapse it into // the `use` below. use self::providers::{DuckDbProvider, QdrantProvider, YamlProvider}; +use crate::sandbox::mcp_credentials; use crate::vault::{Vault, interpolate_secrets}; use anyhow::{Context, Result, anyhow, bail}; @@ -435,7 +436,7 @@ impl Rag { .with_validator(required!("This field is required")) .with_validator(|input: &str| { // Bracketed IPv6 literals would produce a malformed sandbox - // allowedDomains entry, so refuse them at the prompt. + // network allow entry, so refuse them at the prompt. Ok(if input.contains('[') || input.contains(']') { Validation::Invalid( "Bracketed IPv6 literals are not supported; use a hostname.".into(), @@ -568,9 +569,16 @@ impl Rag { rag.save()?; println!("✓ Attached '{name}' → collection '{collection}' on {host}."); - let env_var = rag_env_var_name(name); + let env_var = api_key_entry.as_ref().map(|_| rag_env_var_name(name)); let (header_name, value_format) = driver_auth_header(driver); - generate_rag_sbx_mixin(save_path, &host, name, &env_var, header_name, value_format)?; + generate_rag_sbx_mixin( + save_path, + &host, + name, + env_var.as_deref(), + header_name, + value_format, + )?; Ok(rag) } @@ -1761,48 +1769,57 @@ impl DocumentId { /// byte-for-byte to `spec.yaml` inside a kit dir handed to `sbx create --kit`, /// with no `kind` rewrite — so an envelope-less file can break the launch /// itself, not merely this RAG's traffic. -/// 2. `allowedDomains` entries carry a port; `serviceDomains` keys are bare -/// hostnames. The asymmetry is deliberate. +/// 2. The `service` declared here must be the id the host binds the value +/// under with `sbx secret set`; both derive from the RAG name through +/// `secret_service_id`, so they cannot spell it differently. +/// +/// `api_key_env` is `None` for a store that needs no credential: the host is +/// still allowed, but no binding is declared, because nothing binds a value. fn generate_rag_sbx_mixin( rag_yaml_path: &Path, host: &str, service_name: &str, - env_var: &str, + api_key_env: Option<&str>, header_name: &str, value_format: &str, ) -> Result<()> { - let (bare_host, allowed_domain) = sbx_domain_forms(host); + // The client reaches the store through `normalize_base_url`, so deriving the + // allow entry from that same URL keeps the whitelist and the actual dialled + // port from drifting apart. + let base_url = QdrantProvider::normalize_base_url(host); + let Some(allow_entry) = mcp_credentials::allow_entry_for_url(&base_url) else { + eprintln!( + "Warning: host '{host}' has no representation in the sbx network allow \ + grammar, so no sandbox mixin was written for RAG '{service_name}'. \ + Queries to this RAG will be blocked inside the sandbox." + ); + return Ok(()); + }; + + let credentials = api_key_env + .map(|env_var| mcp_credentials::CredentialEntry { + service: mcp_credentials::secret_service_id(service_name), + description: format!("API key for the attached RAG '{service_name}'"), + api_key: mcp_credentials::ApiKey { + name: env_var.to_string(), + proxy_managed: true, + inject: vec![rag_inject_rule(&allow_entry, header_name, value_format)], + }, + }) + .into_iter() + .collect(); + let mixin_path = rag_yaml_path.with_extension("sbx-mixin.yaml"); - let content = format!( - r#"schemaVersion: "1" -kind: mixin -name: rag-{service_name} -description: > - Auto-generated by the Coyote attach wizard for RAG '{service_name}'. Allows - outbound traffic to its external vector store and tells the sbx proxy which - header to rewrite with the stored credential. Do not edit manually. - -network: - allowedDomains: - - "{allowed_domain}" - serviceDomains: - {bare_host}: {service_name} - serviceAuth: - {service_name}: - headerName: {header_name} - valueFormat: "{value_format}" - -credentials: - sources: - {service_name}: - env: - - {env_var} - -environment: - proxyManaged: - - {env_var} -"# - ); + let content = mcp_credentials::render_mixin_document( + &format!("rag-{service_name}"), + &format!( + "Auto-generated by the Coyote attach wizard for RAG '{service_name}'. Allows \ + outbound traffic to its external vector store and declares the credential the \ + sbx proxy injects into each request. Do not edit manually." + ), + credentials, + &[allow_entry], + )?; fs::write(&mixin_path, &content).with_context(|| { format!( "Failed to write sandbox mixin to '{}'", @@ -1813,34 +1830,28 @@ environment: Ok(()) } -/// Splits a user-supplied host into the two forms sbx needs: -/// `(bare_host, allowed_domain)`. -/// -/// `bare_host` is the `serviceDomains` KEY — hostname only, no scheme, no port. -/// `allowed_domain` is an `allowedDomains` ENTRY — always `host:port`. -/// -/// The rule: emit the host verbatim when it already carries a numeric port, -/// otherwise append the default for the scheme. Stripping the port fails -/// silently — the sandbox just denies the connection, with no compile or test -/// signal. Defaults match `normalize_base_url`, which assumes `http://` when no -/// scheme is given: plain host → 6333, explicit `https://` → 443. -fn sbx_domain_forms(host: &str) -> (String, String) { - let is_https = host.starts_with("https://"); - let hostport = host - .strip_prefix("https://") - .or_else(|| host.strip_prefix("http://")) - .unwrap_or(host) - .trim_end_matches('/') - // A trailing ':' with no digits is a typo, not a port. Trim it so the - // default-port arm cannot emit "host::6333". - .trim_end_matches(':'); - match hostport.rsplit_once(':') { - Some((h, p)) if !p.is_empty() && p.bytes().all(|b| b.is_ascii_digit()) => { - (h.to_string(), hostport.to_string()) +/// Bearer credentials are spelled as a `scheme`, everything else as an explicit +/// `header`; the two are mutually exclusive in the inject grammar. +fn rag_inject_rule( + domain: &str, + header_name: &str, + value_format: &str, +) -> mcp_credentials::InjectRule { + if header_name.eq_ignore_ascii_case("authorization") + && value_format.eq_ignore_ascii_case("Bearer %s") + { + mcp_credentials::InjectRule { + domain: domain.to_string(), + header: None, + format: None, + scheme: Some("bearer".to_string()), } - _ => { - let port = if is_https { 443 } else { 6333 }; - (hostport.to_string(), format!("{hostport}:{port}")) + } else { + mcp_credentials::InjectRule { + domain: domain.to_string(), + header: Some(header_name.to_string()), + format: Some(value_format.to_string()), + scheme: None, } } } @@ -2226,99 +2237,180 @@ mod tests { ); } - /// `allowedDomains` entries are host:PORT; `serviceDomains` keys are bare - /// hostnames. Stripping the port breaks sandbox whitelisting silently — no - /// compile error, no runtime error on the host, just a denied connection - /// inside the sandbox. This test is the only signal. - #[test] - fn sbx_domain_forms_keeps_explicit_ports_and_defaults_the_rest() { - assert_eq!( - sbx_domain_forms("rag.example.com:6333"), - ("rag.example.com".into(), "rag.example.com:6333".into()) - ); - // A non-default explicit port must survive. - assert_eq!( - sbx_domain_forms("rag.example.com:7777").1, - "rag.example.com:7777" - ); - // Bare host → Qdrant's REST default, matching normalize_base_url. - assert_eq!( - sbx_domain_forms("rag.example.com"), - ("rag.example.com".into(), "rag.example.com:6333".into()) - ); - // The scheme is stripped; http keeps 6333. - assert_eq!( - sbx_domain_forms("http://localhost:6333").1, - "localhost:6333" - ); - // https with no port → 443 (the Qdrant Cloud shape). - assert_eq!( - sbx_domain_forms("https://xyz.cloud.qdrant.io"), - ( - "xyz.cloud.qdrant.io".into(), - "xyz.cloud.qdrant.io:443".into() - ) - ); - // A dangling colon is a typo, not a port: trimmed, then defaulted. - assert_eq!( - sbx_domain_forms("rag.example.com:").1, - "rag.example.com:6333" - ); - } - - /// The generated mixin must carry the schema envelope: `wrap_mixin_as_kit` - /// copies it verbatim to `spec.yaml` for `sbx create --kit`, with no `kind` - /// rewrite, so an envelope-less file can break the launch itself. - #[test] - fn generated_sbx_mixin_carries_the_schema_envelope() { + /// Renders a sidecar into a scratch dir and returns its text plus the + /// parsed document. Parsing is not optional: a malformed document is + /// otherwise copied verbatim into `spec.yaml` and only rejected by sbx. + fn render_rag_mixin( + host: &str, + name: &str, + api_key_env: Option<&str>, + header_name: &str, + value_format: &str, + ) -> (String, serde_yaml::Value) { let dir = TempDir::new("mixin"); - let yaml_path = dir.path.join("company-docs.yaml"); + let yaml_path = dir.path.join(format!("{name}.yaml")); generate_rag_sbx_mixin( &yaml_path, - "rag.example.com", - "company-docs", - "COMPANY_DOCS_API_KEY", - "api-key", - "%s", + host, + name, + api_key_env, + header_name, + value_format, ) .unwrap(); - let text = fs::read_to_string(dir.path.join("company-docs.sbx-mixin.yaml")).unwrap(); + let text = fs::read_to_string(dir.path.join(format!("{name}.sbx-mixin.yaml"))).unwrap(); + let parsed = serde_yaml::from_str(&text).unwrap(); + (text, parsed) + } + + fn allow_list(parsed: &serde_yaml::Value) -> Vec { + parsed["permissions"]["network"]["allow"] + .as_sequence() + .expect("a mixin without an allow list whitelists nothing") + .iter() + .map(|v| v.as_str().unwrap().to_string()) + .collect() + } + + /// The generated mixin must carry the kit v2 schema envelope: + /// `wrap_mixin_as_kit` copies it verbatim to `spec.yaml` for + /// `sbx create --kit`, with no `kind` rewrite and no validation on Coyote's + /// side, so a stale envelope breaks the launch itself. + #[test] + fn generated_sbx_mixin_carries_the_schema_envelope() { + let (text, parsed) = render_rag_mixin( + "rag.example.com:6333", + "company-docs", + Some("COMPANY_DOCS_API_KEY"), + "api-key", + "%s", + ); assert!( text.starts_with("schemaVersion:"), "envelope must come first:\n{text}" ); - assert!( - text.contains("kind: mixin"), - "kind must be `mixin`, not `sandbox`" - ); - assert!(text.contains("name: rag-company-docs")); - assert!(text.contains("description:")); - - // It must parse as YAML at all — a broken format! escape is invisible otherwise. - let parsed: serde_yaml::Value = serde_yaml::from_str(&text).unwrap(); + // Quoted string, never the numeric 2 — the kit rejects an int here. + assert_eq!(parsed["schemaVersion"].as_str(), Some("2")); assert_eq!(parsed["kind"].as_str(), Some("mixin")); - // The list entry carries the port; the map key does not. + assert_eq!(parsed["name"].as_str(), Some("rag-company-docs")); + assert!(parsed["description"].as_str().is_some()); + + // https/443 is the only bare-host case; 6333 must carry its port, and + // the bare host must NOT also be listed. + assert_eq!(allow_list(&parsed), vec!["rag.example.com:6333"]); + + let credential = &parsed["credentials"][0]; + assert_eq!(credential["service"].as_str(), Some("company-docs")); assert_eq!( - parsed["network"]["allowedDomains"][0].as_str(), - Some("rag.example.com:6333") - ); - assert_eq!( - parsed["network"]["serviceDomains"]["rag.example.com"].as_str(), - Some("company-docs") - ); - // The proxy rewrites this header with the credential it holds. - assert_eq!( - parsed["network"]["serviceAuth"]["company-docs"]["headerName"].as_str(), - Some("api-key") - ); - assert_eq!( - parsed["credentials"]["sources"]["company-docs"]["env"][0].as_str(), + credential["apiKey"]["name"].as_str(), Some("COMPANY_DOCS_API_KEY") ); + assert_eq!(credential["apiKey"]["proxyManaged"].as_bool(), Some(true)); + + let inject = &credential["apiKey"]["inject"][0]; + assert_eq!(inject["domain"].as_str(), Some("rag.example.com:6333")); + assert_eq!(inject["header"].as_str(), Some("api-key")); + assert_eq!(inject["format"].as_str(), Some("%s")); + + // sbx does not derive allow entries from inject rules; an inject + // domain that is not allowed is a dead rule. + assert!( + allow_list(&parsed).contains(&inject["domain"].as_str().unwrap().to_string()), + "every inject domain must also appear in allow:\n{text}" + ); + + // The v1 vocabulary is gone, not merely unused. + for dead in ["allowedDomains", "serviceDomains", "serviceAuth"] { + assert!(!text.contains(dead), "v1 key '{dead}' survived:\n{text}"); + } + assert!( + parsed["network"].is_null(), + "v1 top-level `network` survived:\n{text}" + ); + assert!( + parsed["environment"].is_null(), + "v1 `environment.proxyManaged` survived:\n{text}" + ); + } + + /// The allow entry has to name the port the client actually dials, which is + /// whatever `normalize_base_url` resolves to — not a Qdrant-specific guess. + #[test] + fn generated_sbx_mixin_allows_the_port_the_client_dials() { + let cases = [ + // No scheme means http, and http means port 80 — normalize_base_url + // does not silently append Qdrant's 6333. + ("rag.example.com", "rag.example.com:80"), + ("rag.example.com:7777", "rag.example.com:7777"), + ("http://localhost:6333", "localhost:6333"), + // https on the default port is the one bare-host case. + ("https://xyz.cloud.qdrant.io", "xyz.cloud.qdrant.io"), + ( + "https://xyz.cloud.qdrant.io:6333", + "xyz.cloud.qdrant.io:6333", + ), + ]; + for (host, expected) in cases { + let (_, parsed) = render_rag_mixin(host, "docs", Some("DOCS_API_KEY"), "api-key", "%s"); + assert_eq!( + allow_list(&parsed), + vec![expected.to_string()], + "host {host}" + ); + } + } + + /// `Authorization: Bearer` is spelled as a scheme; `header` and `scheme` are + /// mutually exclusive in the inject grammar. + #[test] + fn generated_sbx_mixin_spells_bearer_as_a_scheme() { + let (_, parsed) = render_rag_mixin( + "https://store.example.com", + "docs", + Some("DOCS_API_KEY"), + "Authorization", + "Bearer %s", + ); + let inject = &parsed["credentials"][0]["apiKey"]["inject"][0]; + assert_eq!(inject["scheme"].as_str(), Some("bearer")); + assert!(inject["header"].is_null()); + assert!(inject["format"].is_null()); + } + + /// The bind in `inject_rag_secrets` and the `service` declared here both run + /// the RAG name through `secret_service_id`. If they disagreed, the proxy + /// would hold a value under one id and an inject rule under another, and the + /// header would never be rewritten. + #[test] + fn generated_sbx_mixin_service_id_matches_the_host_side_bind() { + let (_, parsed) = render_rag_mixin( + "https://store.example.com", + "My_Docs", + Some("MY_DOCS_API_KEY"), + "api-key", + "%s", + ); assert_eq!( - parsed["environment"]["proxyManaged"][0].as_str(), - Some("COMPANY_DOCS_API_KEY") + parsed["credentials"][0]["service"].as_str(), + Some(crate::sandbox::mcp_credentials::secret_service_id("My_Docs").as_str()) + ); + assert_eq!( + parsed["credentials"][0]["service"].as_str(), + Some("my-docs") + ); + } + + /// A store with no API key still needs egress, but declaring a credential + /// nothing ever binds would leave sbx waiting on a binding that never comes. + #[test] + fn generated_sbx_mixin_omits_credentials_when_there_is_no_api_key() { + let (text, parsed) = + render_rag_mixin("https://store.example.com", "docs", None, "api-key", "%s"); + assert_eq!(allow_list(&parsed), vec!["store.example.com"]); + assert!( + parsed["credentials"].is_null(), + "no key means no credential declaration:\n{text}" ); } diff --git a/src/rag/providers/qdrant.rs b/src/rag/providers/qdrant.rs index ffff33a..a2d2c34 100644 --- a/src/rag/providers/qdrant.rs +++ b/src/rag/providers/qdrant.rs @@ -89,7 +89,7 @@ impl QdrantProvider { .context("Failed to build reqwest client") } - fn normalize_base_url(host: &str) -> String { + pub(crate) fn normalize_base_url(host: &str) -> String { if host.starts_with("http://") || host.starts_with("https://") { host.to_string() } else { diff --git a/src/sandbox/mcp_credentials.rs b/src/sandbox/mcp_credentials.rs index 0132e21..a16d8e6 100644 --- a/src/sandbox/mcp_credentials.rs +++ b/src/sandbox/mcp_credentials.rs @@ -387,7 +387,11 @@ pub(crate) fn collect_server_allow_entries( out.into_iter().collect() } -fn allow_entry_for_url(raw: &str) -> Option { +/// The single definition of the sbx kit v2 allow-list entry grammar: https on +/// the default port yields a bare host, anything else is spelled `host:port`. +/// Bracketed IPv6 hosts and non-http(s) schemes have no representation in the +/// grammar and yield `None`. +pub(crate) fn allow_entry_for_url(raw: &str) -> Option { let url = Url::parse(raw).ok()?; let scheme = url.scheme(); if scheme != "https" && scheme != "http" { @@ -426,8 +430,8 @@ fn placeholders(text: &str) -> Result> { struct CredentialsMixin { schema_version: &'static str, kind: &'static str, - name: &'static str, - description: &'static str, + name: String, + description: String, #[serde(skip_serializing_if = "Vec::is_empty")] credentials: Vec, #[serde(skip_serializing_if = "Option::is_none")] @@ -435,20 +439,20 @@ struct CredentialsMixin { } #[derive(Serialize)] -struct CredentialEntry { - service: String, - description: String, +pub(crate) struct CredentialEntry { + pub service: String, + pub description: String, #[serde(rename = "apiKey")] - api_key: ApiKey, + pub api_key: ApiKey, } #[derive(Serialize)] #[serde(rename_all = "camelCase")] -struct ApiKey { - name: String, - proxy_managed: bool, +pub(crate) struct ApiKey { + pub name: String, + pub proxy_managed: bool, #[serde(skip_serializing_if = "Vec::is_empty")] - inject: Vec, + pub inject: Vec, } #[derive(Serialize)] @@ -461,40 +465,30 @@ struct Network { allow: Vec, } -pub(crate) fn render_mixin_yaml( - credentials: &[CredentialSpec], - server_allow_entries: &[String], +/// Serializes one sbx kit v2 mixin document. +/// +/// Every `inject[].domain` is unioned into `permissions.network.allow`: sbx +/// does not derive allow entries from inject rules, so a rule whose domain is +/// not allowed would be dead. Enforcing it here keeps the invariant in one +/// place for every mixin Coyote generates. +pub(crate) fn render_mixin_document( + name: &str, + description: &str, + credentials: Vec, + extra_allow_entries: &[String], ) -> Result { let mut allow: BTreeSet = credentials .iter() - .flat_map(|c| c.inject.iter().map(|r| r.domain.clone())) + .flat_map(|c| c.api_key.inject.iter().map(|r| r.domain.clone())) .collect(); - allow.extend(server_allow_entries.iter().cloned()); + allow.extend(extra_allow_entries.iter().cloned()); let mixin = CredentialsMixin { schema_version: "2", kind: "mixin", - name: MCP_MIXIN_NAME, - description: "Auto-generated by Coyote at launch: allows network egress to the user's \ - remote MCP servers and declares their credentials so Docker Sandboxes \ - binds them (bindings are approved on first interactive run). Values are \ - pre-seeded from Coyote's vault via `sbx secret set`.", - credentials: credentials - .iter() - .map(|c| CredentialEntry { - service: c.service_id.clone(), - description: format!( - "Coyote vault secret '{}', used by MCP server(s) {}", - c.secret_name, - quoted_list(&c.servers) - ), - api_key: ApiKey { - name: c.env_var.clone(), - proxy_managed: c.proxy_managed, - inject: c.inject.clone(), - }, - }) - .collect(), + name: name.to_string(), + description: description.to_string(), + credentials, permissions: (!allow.is_empty()).then(|| Permissions { network: Network { allow: allow.into_iter().collect(), @@ -502,7 +496,39 @@ pub(crate) fn render_mixin_yaml( }), }; - serde_yaml::to_string(&mixin).context("Failed to serialize generated MCP credentials mixin") + serde_yaml::to_string(&mixin).context("Failed to serialize generated sandbox mixin") +} + +pub(crate) fn render_mixin_yaml( + credentials: &[CredentialSpec], + server_allow_entries: &[String], +) -> Result { + let entries = credentials + .iter() + .map(|c| CredentialEntry { + service: c.service_id.clone(), + description: format!( + "Coyote vault secret '{}', used by MCP server(s) {}", + c.secret_name, + quoted_list(&c.servers) + ), + api_key: ApiKey { + name: c.env_var.clone(), + proxy_managed: c.proxy_managed, + inject: c.inject.clone(), + }, + }) + .collect(); + + render_mixin_document( + MCP_MIXIN_NAME, + "Auto-generated by Coyote at launch: allows network egress to the user's remote MCP \ + servers and declares their credentials so Docker Sandboxes binds them (bindings are \ + approved on first interactive run). Values are pre-seeded from Coyote's vault via \ + `sbx secret set`.", + entries, + server_allow_entries, + ) } #[cfg(test)] diff --git a/src/sandbox/mod.rs b/src/sandbox/mod.rs index 8ac1736..4fb9e08 100644 --- a/src/sandbox/mod.rs +++ b/src/sandbox/mod.rs @@ -10,7 +10,7 @@ use std::path::{Path, PathBuf}; use std::process::{Command, Stdio}; use which::which; -mod mcp_credentials; +pub(crate) mod mcp_credentials; mod mixins; pub(crate) use mcp_credentials::sandbox_secret_env_var; @@ -346,7 +346,12 @@ fn inject_rag_secrets(vault: &Vault, registered: &HashSet) -> Result<()> let Some(placeholder) = data.driver_config.get("api_key") else { continue; }; - if registered.contains(&stem) { + // The sidecar mixin declares `credentials[].service` under the same + // derivation, so the bound value and the inject rule that consumes it + // always name the same service. Passing the raw stem here would produce + // an id sbx rejects for any RAG whose name is not already a valid id. + let service_id = mcp_credentials::secret_service_id(&stem); + if service_id.is_empty() || registered.contains(&service_id) { continue; } let secret_name = placeholder @@ -358,7 +363,7 @@ fn inject_rag_secrets(vault: &Vault, registered: &HashSet) -> Result<()> // is recoverable without a restart. match vault.get_secret(secret_name, false) { Ok(secret_value) => { - sbx_secret_set(&stem, &secret_value) + sbx_secret_set(&service_id, &secret_value) .context("Failed to register RAG secret with sbx")?; } Err(e) => {