fix(rag): emit an sbx kit v2 mixin and declare RAG credentials to the proxy
The RAG attach sidecar was written against the sbx kit v1 spec and still emitted schemaVersion "1" with network.allowedDomains, network.serviceDomains, network.serviceAuth, credentials.sources.<n>.env and environment.proxyManaged. Every one of those keys was removed in kit v2. Coyote does not validate mixins, it copies them byte-for-byte into spec.yaml, so the invalid document surfaced only as an opaque sbx failure with no indication of which mixin caused it. generate_rag_sbx_mixin now builds the document from the shared serializer structs instead of a format! string, which is how the envelope drifted unnoticed in the first place. render_mixin_yaml and the RAG sidecar both go through a new render_mixin_document, giving one definition of the envelope and one enforcement point for the rule that every inject domain must also appear in permissions.network.allow. Fix an auth bug the port exposed: inject_rag_secrets bound the API key with sbx secret set, but nothing ever emitted a matching credentials entry, so the proxy held a value with no inject rule and never rewrote the auth header. An attached RAG credential silently did not work inside the sandbox. The sidecar now declares that credential; a RAG with no API key declares none while still receiving egress. Fix the service id: the bind passed the raw file stem instead of routing it through secret_service_id, so a RAG named My_Docs produced an illegal id. The bind and the generated credentials service now share that derivation and cannot disagree. Retire sbx_domain_forms in favour of allow_entry_for_url, now pub(crate). It emitted both a bare host and host:port because v1 serviceDomains needed a bare key; v2 has no such need, so the extra entry is simply wrong. It also defaulted a schemeless host to port 6333 while normalize_base_url resolves it to http and port 80, meaning the allow entry named a port the client never dialled.
This commit is contained in:
+234
-142
@@ -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<String> {
|
||||
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}"
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user