diff --git a/crates/biorouter/src/privacy/config_keys.rs b/crates/biorouter/src/privacy/config_keys.rs index 71f8c0da1..7d5fbe719 100644 --- a/crates/biorouter/src/privacy/config_keys.rs +++ b/crates/biorouter/src/privacy/config_keys.rs @@ -54,13 +54,18 @@ pub const NOT_CAPABILITY_CONFIG_KEYS: &[(&str, &str)] = &[ ("LLAMACPP_TIMEOUT", "transport timeout"), ("LLAMACPP_STARTUP_TIMEOUT", "sidecar readiness deadline"), ("LLAMACPP_CONTEXT_SIZE", "token budget"), - // ⚠ The four endpoint keys below MOVE where a Private-badged provider sends - // traffic, but they cannot RAISE a tier: Task 5 name-keys versa_azure and - // versa_bedrock Private regardless of endpoint, and azure.rs ships the - // UCSF gateway as a PUBLIC provider's default for the same reason. - // Pointing a private-badged provider off-site is a real and different - // problem — it belongs to Task 5's tier definition and to Open question 5, - // not to DR-16 — and it is recorded here rather than left unstated. + // ⚠ The two endpoint keys below MOVE where a Private-badged provider sends + // traffic, and since `e2e4eb9d` that moves its tier as well: `tier()` + // follows the endpoint an instance resolved (`ucsf_gateway_tier`), so an + // off-site value demotes it to Public, and deleting that value restores + // Private. These rows used to say the keys "cannot RAISE a tier" because + // Task 5 name-keyed versa_* Private regardless of endpoint, and that + // stopped being true. The classification rests on this instead: the only + // value that reads Private is the UCSF gateway's own host, so no write can + // make an off-site endpoint look Private, and a raise through one of these + // keys is always a return to the institution's gateway. Whether even that + // raise should be a user act, as it is for `OLLAMA_HOST`, is an open DR-16 + // question, recorded here rather than left unstated. // // Versa Azure's three overrides, in its own namespace. It used to share the // public `azure_openai` card's `AZURE_OPENAI_*` keys, which went wrong both @@ -74,15 +79,25 @@ pub const NOT_CAPABILITY_CONFIG_KEYS: &[(&str, &str)] = &[ // tier-input file, because `azure_openai` is Public wherever it points. ( "VERSA_AZURE_ENDPOINT", - "moves a Private provider's endpoint; does not raise a tier (see Task 5)", + "moves a Private provider's endpoint; only the UCSF gateway reads Private (see above)", ), ("VERSA_AZURE_DEPLOYMENT_NAME", "deployment selection"), ("VERSA_AZURE_API_VERSION", "wire version"), + // Versa Bedrock's two overrides, in its own namespace since 2026-09-11. It + // used to declare and read the public Amazon Bedrock card's `AWS_REGION` and + // an `AWS_ENDPOINT_URL_BEDROCK` key, then fall back to the process + // environment, so the public side's values steered Versa and a Versa setup + // configured the public card. No tier-input file reads an `AWS_*` key now, + // so none has a row; `bedrock.rs` still reads them and is not a tier-input + // file, because `aws_bedrock` is Public wherever it points. ( - "AWS_ENDPOINT_URL_BEDROCK", - "moves a Private provider's endpoint; does not raise a tier (see Task 5)", + "VERSA_BEDROCK_ENDPOINT", + "moves a Private provider's endpoint; only the UCSF gateway reads Private (see above)", + ), + ( + "VERSA_BEDROCK_REGION", + "SigV4 signing region; the endpoint, not the region, decides where a request goes", ), - ("AWS_REGION", "region selection"), ("BEDROCK_MAX_RETRIES", "retry policy"), ("BEDROCK_INITIAL_RETRY_INTERVAL_MS", "retry policy"), ("BEDROCK_BACKOFF_MULTIPLIER", "retry policy"), diff --git a/crates/biorouter/src/providers/bedrock.rs b/crates/biorouter/src/providers/bedrock.rs index 3bfbed5d8..d913ffbdc 100644 --- a/crates/biorouter/src/providers/bedrock.rs +++ b/crates/biorouter/src/providers/bedrock.rs @@ -83,15 +83,23 @@ impl BedrockProvider { set_aws_env_vars(config.all_values()); set_aws_env_vars(config.all_secrets()); - // Normalize AWS_ENDPOINT_URL_BEDROCK → AWS_ENDPOINT_URL_BEDROCK_RUNTIME. - // The AWS SDK for Rust reads the service-specific key AWS_ENDPOINT_URL_BEDROCK_RUNTIME, - // but users (and older configs) often set the shorter AWS_ENDPOINT_URL_BEDROCK. - // Accept either: if only the short form is set, promote it to the correct key. - if std::env::var("AWS_ENDPOINT_URL_BEDROCK_RUNTIME").is_err() { - if let Ok(url) = std::env::var("AWS_ENDPOINT_URL_BEDROCK") { - std::env::set_var("AWS_ENDPOINT_URL_BEDROCK_RUNTIME", url); - } - } + // ⚠ `AWS_ENDPOINT_URL_BEDROCK` is not an endpoint for this provider. The + // AWS SDK aims Bedrock Runtime at `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`, + // from the environment or from `config.yaml` through the export above, + // or at an AWS profile's `services` section; that is how a VPC endpoint + // or a proxy is meant to be set. `AWS_ENDPOINT_URL_BEDROCK` is the name + // the SDK derives for a different service, the Bedrock control plane. + // + // This used to promote it to `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`, added + // on 2026-04-12 for configs and setup scripts that used the short name, + // a month before Versa Bedrock existed. But every Biorouter surface that + // has written that key wrote it for Versa Bedrock: its onboarding card on + // every connect, its Settings form on every save, `biorouter configure` + // when asked to. So once Versa was set up, the promotion made UCSF's + // gateway THIS provider's endpoint, and the user's own AWS-signed + // requests went there and were refused. Versa reads its own + // `VERSA_BEDROCK_ENDPOINT` now, but installs keep the old key, so it has + // to be ignored here, not merely left unwritten (2026-09-11). // Use load_defaults() which supports AWS SSO, profiles, and environment variables let mut loader = aws_config::defaults(aws_config::BehaviorVersion::latest()); @@ -137,6 +145,19 @@ impl BedrockProvider { }) } + /// The same client with only its HTTP transport replaced. Everything + /// `from_env` resolved — endpoint, region, credentials — is kept, so a + /// request captured through it is the request production would have sent. + #[cfg(test)] + pub(crate) fn with_http_client( + mut self, + http_client: impl aws_sdk_bedrockruntime::config::HttpClient + 'static, + ) -> Self { + let config = self.client.config().to_builder().http_client(http_client); + self.client = Client::from_conf(config.build()); + self + } + fn load_retry_config(config: &crate::config::Config) -> RetryConfig { let max_retries = config .get_param::("BEDROCK_MAX_RETRIES") diff --git a/crates/biorouter/src/providers/bedrock_namespace_tests.rs b/crates/biorouter/src/providers/bedrock_namespace_tests.rs new file mode 100644 index 000000000..a26ac8b0e --- /dev/null +++ b/crates/biorouter/src/providers/bedrock_namespace_tests.rs @@ -0,0 +1,544 @@ +//! The public `aws_bedrock` provider and UCSF's private `versa_bedrock` must not +//! steer each other. The Bedrock twin of `versa_azure`'s `routing_tests`. +//! +//! Declared in `providers/mod.rs` as +//! `#[cfg(all(test, feature = "aws-providers"))] mod bedrock_namespace_tests;`. +//! `aws-providers` is a default feature, so a plain `cargo test -p biorouter +//! --lib` runs every row here. A `--no-default-features` build compiles neither +//! provider, and this module goes with them. +//! +//! **What they shared.** Versa declared the public card's `AWS_REGION` and an +//! `AWS_ENDPOINT_URL_BEDROCK` key as its own, read both (and then the process +//! environment) as overrides, and its setup surfaces wrote both. `bedrock.rs` +//! exports every `AWS_*` config value and secret into the process environment +//! and promoted `AWS_ENDPOINT_URL_BEDROCK` to the variable the AWS SDK reads, so +//! the UCSF gateway Versa persisted became the public provider's endpoint. And +//! the SDK reads `AWS_BEARER_TOKEN_BEDROCK` from the environment on its own, and +//! authenticates with it instead of signing whenever it is there. +//! +//! **How each row measures.** Every provider is built the way production builds +//! it, through `from_env`, and only its HTTP transport is then swapped for the +//! SDK's own capture client (`with_http_client`). So every assertion is on the +//! request production would have sent: its host, its path, its `Authorization` +//! header. Nothing leaves the process, even on the rows whose bug aims a request +//! at public AWS. The stand-in is the capture client rather than wiremock +//! because the thing under test is the host the SDK resolved, and aiming the +//! endpoint at a local server would overwrite exactly that. +//! +//! The config rows pin their inputs with `with_config_overrides`, which +//! `get_param` consults before the environment and the file. The environment +//! rows cannot: the SDK reads the environment through its own shim, and the +//! public provider's `std::env::set_var` is part of what is under test. Calling +//! it in this multi-threaded binary is unsound, and its writes would leak into +//! every test running beside it. So those rows re-execute this test binary and +//! run in a child process that STARTS with the environment the scenario +//! describes and a config root of its own, as +//! `workflow::local_workflows::tests::listing_workflows_survives_a_deleted_working_directory` +//! does for a deleted working directory. + +use super::base::Provider; +use super::bedrock::{BedrockProvider, BEDROCK_DEFAULT_MODEL}; +use super::versa_bedrock::{ + VersaBedrockProvider, VERSA_BEDROCK_DEFAULT_ENDPOINT, VERSA_BEDROCK_DEFAULT_MODEL, + VERSA_BEDROCK_DEFAULT_REGION, +}; +use crate::conversation::message::Message; +use crate::model::ModelConfig; +use crate::privacy::ProviderTier; +use aws_smithy_http_client::test_util::capture_request; +use std::collections::HashMap; + +const VERSA_ACCESS_KEY: &str = "VERSATESTACCESSKEY"; +const VERSA_SECRET_KEY: &str = "versa-test-secret-key"; +const PUBLIC_ACCESS_KEY: &str = "PUBLICTESTACCESSKEY"; +const PUBLIC_SECRET_KEY: &str = "public-test-secret-key"; +/// The public card's Bedrock API key, in the variable the AWS SDK reads it from. +const PUBLIC_BEARER_TOKEN: &str = "public-bedrock-api-key"; +/// What the public card could point at: its user's own AWS region. +const PUBLIC_ENDPOINT: &str = "https://bedrock-runtime.eu-central-1.amazonaws.com"; +const PUBLIC_REGION: &str = "eu-central-1"; +const UCSF_GATEWAY_HOST: &str = "unified-api.ucsf.edu"; + +/// One request, as it would have left the machine. +#[derive(Debug, serde::Serialize, serde::Deserialize)] +struct Sent { + host: String, + path: String, + authorization: String, +} + +impl Sent { + fn of(request: &aws_smithy_runtime_api::client::orchestrator::HttpRequest) -> Self { + let url = url::Url::parse(request.uri()).expect("the SDK sends an absolute URI"); + Self { + host: url.host_str().unwrap_or_default().to_string(), + path: url.path().to_string(), + authorization: request + .headers() + .get("authorization") + .unwrap_or_default() + .to_string(), + } + } + + /// `(access key id, signing region)`, if the request was SigV4-signed. + fn signed_by(&self) -> Option<(&str, &str)> { + let credential = self + .authorization + .strip_prefix("AWS4-HMAC-SHA256 Credential=")?; + let mut scope = credential.split(',').next()?.split('/'); + let key = scope.next()?; + let _date = scope.next()?; + let region = scope.next()?; + Some((key, region)) + } +} + +/// What a row observed: for Versa, the endpoint, region and tier the instance +/// resolved; for either provider, the request it sent. +#[derive(Debug, serde::Serialize, serde::Deserialize)] +struct Observed { + resolved: Option<(String, String, String)>, + sent: Sent, +} + +/// Send one turn and return the request it made. The stand-in answers 200 with +/// an empty body, which does not parse, so the turn fails AFTER the request is +/// made — and the request is what is measured. Every row builds with +/// `BEDROCK_MAX_RETRIES=0`, because the stand-in answers once. +async fn turn(provider: &dyn Provider) { + let _ = provider + .complete("system", &[Message::user().with_text("hello")], &[]) + .await; +} + +async fn versa_sent(provider: VersaBedrockProvider) -> Sent { + let (http, captured) = capture_request(None); + let provider = provider.with_http_client(http); + turn(&provider).await; + Sent::of(&captured.expect_request()) +} + +async fn public_sent(provider: BedrockProvider) -> Sent { + let (http, captured) = capture_request(None); + let provider = provider.with_http_client(http); + turn(&provider).await; + Sent::of(&captured.expect_request()) +} + +/// Versa's credentials and Versa's own two overrides as given, blank meaning +/// absent, so the machine running the suite cannot leak its own configuration +/// into what is measured. +fn versa_config(endpoint: &str, region: &str) -> HashMap { + HashMap::from([ + ( + "VERSA_BEDROCK_ACCESS_KEY_ID".into(), + VERSA_ACCESS_KEY.into(), + ), + ( + "VERSA_BEDROCK_SECRET_ACCESS_KEY".into(), + VERSA_SECRET_KEY.into(), + ), + ("VERSA_BEDROCK_ENDPOINT".into(), endpoint.into()), + ("VERSA_BEDROCK_REGION".into(), region.into()), + ("BEDROCK_MAX_RETRIES".into(), "0".into()), + ]) +} + +async fn versa_bound(overrides: HashMap) -> VersaBedrockProvider { + crate::config::with_config_overrides( + overrides, + VersaBedrockProvider::from_env(ModelConfig::new_or_fail(VERSA_BEDROCK_DEFAULT_MODEL)), + ) + .await + .unwrap_or_else(|e| panic!("Versa Bedrock must construct from its credentials alone: {e}")) +} + +/// The endpoint and region a bound instance will be restored with, and its tier. +fn resolved(provider: &VersaBedrockProvider) -> (String, String, String) { + let binding = serde_json::to_value(provider.restore_binding()).unwrap(); + ( + binding["endpoint"].as_str().unwrap_or_default().to_string(), + binding["region"].as_str().unwrap_or_default().to_string(), + format!("{:?}", provider.tier()), + ) +} + +fn shipped() -> (String, String, String) { + ( + VERSA_BEDROCK_DEFAULT_ENDPOINT.to_string(), + VERSA_BEDROCK_DEFAULT_REGION.to_string(), + format!("{:?}", ProviderTier::Private), + ) +} + +// ------------------------------------------------------------------ the rule + +/// Configuring UCSF's PRIVATE Versa Bedrock must not configure the PUBLIC +/// Amazon Bedrock card, or hand it a value. +/// +/// `aws_bedrock` declares two keys, both required and both defaulted, and for +/// such a provider `check_provider_configured` says Configured as soon as EITHER +/// is in `config.yaml`. Versa declared one of them, `AWS_REGION`, and its setup +/// surfaces persisted it: the Settings form seeds a declared key's default and +/// `DefaultSubmitHandler` submits it, and the onboarding card wrote it on every +/// connect. Every other `AWS_*` key belongs to the public side as well, declared +/// or not: `bedrock.rs` and `sagemaker_tgi.rs` export each one into the process +/// environment, where the AWS SDK reads them. So this asserts the namespace, not +/// the one key that happened to leak. +#[test] +fn versa_declares_no_key_the_public_bedrock_provider_reads() { + let versa = VersaBedrockProvider::metadata(); + let public = BedrockProvider::metadata(); + let public_keys: Vec<&str> = public + .config_keys + .iter() + .map(|key| key.name.as_str()) + .collect(); + assert!( + public.config_keys.iter().any(|key| key.required), + "if the public provider stops having a key its configured-check turns on, \ + this test is vacuous; re-derive it rather than deleting it" + ); + + let versa_keys = versa.config_keys.iter().map(|key| key.name.as_str()); + let shared: Vec<&str> = versa_keys + .clone() + .filter(|name| public_keys.contains(name)) + .collect(); + let outside: Vec<&str> = versa_keys + .filter(|name| !name.starts_with("VERSA_BEDROCK_")) + .collect(); + assert!( + shared.is_empty() && outside.is_empty(), + "versa_bedrock declares {shared:?}, which the PUBLIC aws_bedrock provider \ + declares too, so a Versa setup marks that card Configured and hands it the \ + value. It declares {outside:?} outside its own namespace, and every \ + `AWS_*` key is the public providers' too: `bedrock.rs` exports each one \ + into the process environment, where the AWS SDK reads it. Two providers \ + of different privacy tiers must not share a config key." + ); +} + +// ------------------------------------------------------ public card → Versa + +/// What the public Amazon Bedrock card, or `bedrock.rs`'s export of it, leaves +/// where Versa used to look: its user's own AWS region, and an endpoint in it. +/// Versa read both whenever its own were unset. Its requests, signed with +/// UCSF-issued keys, then went to that user's AWS region, which refused the +/// keys, and the instance turned Public. The region alone was enough to sign +/// every request for a region other than the gateway's. +#[tokio::test] +async fn the_public_cards_endpoint_and_region_never_reach_versa() { + let mut public_card = versa_config("", ""); + public_card.insert("AWS_ENDPOINT_URL_BEDROCK".into(), PUBLIC_ENDPOINT.into()); + public_card.insert("AWS_REGION".into(), PUBLIC_REGION.into()); + + let versa = versa_bound(public_card).await; + assert_eq!( + resolved(&versa), + shipped(), + "the public Amazon Bedrock card's endpoint or region reached a Versa chat" + ); + + let sent = versa_sent(versa).await; + assert_eq!(sent.host, UCSF_GATEWAY_HOST, "{sent:?}"); + assert!(sent.path.starts_with("/general/awsai/model/"), "{sent:?}"); + assert_eq!( + sent.signed_by(), + Some((VERSA_ACCESS_KEY, VERSA_BEDROCK_DEFAULT_REGION)), + "{sent:?}" + ); +} + +/// The escape hatch survives, in Versa's own namespace. An operator can still +/// repoint the endpoint or the region, a blank value still means the shipped +/// default, and an endpoint off the gateway still demotes the instance: the +/// demotion guards a key anyone can write, not only the public card. +#[tokio::test] +async fn versas_own_overrides_still_steer_it() { + let blank = versa_bound(versa_config(" ", "")).await; + assert_eq!(resolved(&blank), shipped(), "blank must mean the default"); + + let repointed = "https://unified-api.ucsf.edu/general/awsai-v2"; + let custom = versa_bound(versa_config(repointed, "us-east-2")).await; + assert_eq!( + resolved(&custom), + ( + repointed.to_string(), + "us-east-2".to_string(), + format!("{:?}", ProviderTier::Private) + ) + ); + let sent = versa_sent(custom).await; + assert_eq!(sent.host, UCSF_GATEWAY_HOST, "{sent:?}"); + assert!( + sent.path.starts_with("/general/awsai-v2/model/"), + "{sent:?}" + ); + assert_eq!( + sent.signed_by(), + Some((VERSA_ACCESS_KEY, "us-east-2")), + "{sent:?}" + ); + + let off_site = versa_bound(versa_config(PUBLIC_ENDPOINT, "")).await; + assert_eq!( + resolved(&off_site).2, + format!("{:?}", ProviderTier::Public), + "an endpoint off the UCSF gateway must demote the instance" + ); +} + +/// The public card's Bedrock API key, alone in the environment, where the AWS +/// SDK's own documentation tells its user to put it. +/// +/// This one reaches past Versa's own code. `AWS_BEARER_TOKEN_BEDROCK` is the +/// SDK's variable for a Bedrock API key; the SDK reads it itself and, finding +/// it, authenticates with that bearer token instead of signing. So a Versa chat +/// that looked entirely right, with the UCSF gateway, the gateway's region and a +/// Private tier, sent the public card's API key to UCSF in its `Authorization` +/// header, and Versa's own keys signed nothing. +#[tokio::test] +async fn the_public_cards_api_key_never_rides_on_a_versa_request() { + const SCENARIO: &str = "versa-beside-a-bedrock-api-key"; + if child_scenario().as_deref() == Some(SCENARIO) { + report_versa().await; + return; + } + + let observed = run_child( + "the_public_cards_api_key_never_rides_on_a_versa_request", + SCENARIO, + "{}\n", + &[("AWS_BEARER_TOKEN_BEDROCK", PUBLIC_BEARER_TOKEN)], + ); + assert_signed_by_versa_for_the_gateway(&observed); +} + +/// Everything the environment can hold for the PUBLIC side, all at once, and +/// none of it may steer a Versa request: what `bedrock.rs` exports and promotes, +/// what a shell holds for the AWS CLI, and the public card's own credentials. +#[tokio::test] +async fn nothing_in_the_process_environment_steers_versa() { + const SCENARIO: &str = "versa-in-a-public-environment"; + if child_scenario().as_deref() == Some(SCENARIO) { + report_versa().await; + return; + } + + let observed = run_child( + "nothing_in_the_process_environment_steers_versa", + SCENARIO, + "{}\n", + &[ + ("AWS_ENDPOINT_URL_BEDROCK", PUBLIC_ENDPOINT), + ("AWS_ENDPOINT_URL_BEDROCK_RUNTIME", PUBLIC_ENDPOINT), + ("AWS_REGION", PUBLIC_REGION), + ("AWS_BEARER_TOKEN_BEDROCK", PUBLIC_BEARER_TOKEN), + ("AWS_ACCESS_KEY_ID", PUBLIC_ACCESS_KEY), + ("AWS_SECRET_ACCESS_KEY", PUBLIC_SECRET_KEY), + ], + ); + assert_signed_by_versa_for_the_gateway(&observed); +} + +/// The child half of the two rows above: a Versa chat bound with its +/// credentials and nothing else, reported with the one request it sent. +async fn report_versa() { + let versa = versa_bound(versa_config("", "")).await; + report(Observed { + resolved: Some(resolved(&versa)), + sent: versa_sent(versa).await, + }); +} + +/// The shipped endpoint, region and tier, and a request to the gateway signed +/// with Versa's own keys for the gateway's region, carrying no bearer token. One +/// comparison, so a failure shows every part of the request at once. +fn assert_signed_by_versa_for_the_gateway(observed: &Observed) { + let sent = &observed.sent; + assert_eq!( + ( + observed.resolved.clone(), + sent.host.as_str(), + sent.signed_by(), + ), + ( + Some(shipped()), + UCSF_GATEWAY_HOST, + Some((VERSA_ACCESS_KEY, VERSA_BEDROCK_DEFAULT_REGION)), + ), + "the process environment steered a Versa request: {sent:?}" + ); + assert!( + !sent.authorization.contains(PUBLIC_BEARER_TOKEN), + "the public card's API key rode along on a Versa request: {sent:?}" + ); +} + +// ------------------------------------------------------ Versa → public card + +/// The other direction. Versa's setup persisted `AWS_ENDPOINT_URL_BEDROCK` +/// pointing at the UCSF gateway, and `bedrock.rs` promoted that key to the +/// variable the SDK reads. So once the PUBLIC provider was built, it sent the +/// user's own AWS-signed requests to UCSF's gateway, which refused them. +/// Existing installs keep that key, so the public provider has to ignore it; it +/// is not enough to stop writing it. +#[tokio::test] +async fn versas_persisted_endpoint_never_becomes_the_public_providers() { + const SCENARIO: &str = "public-after-a-versa-setup"; + if child_scenario().as_deref() == Some(SCENARIO) { + report(Observed { + resolved: None, + sent: public_sent(public_bound().await).await, + }); + return; + } + + // Exactly what the Versa Bedrock onboarding card wrote on every connect. + let config_yaml = format!( + "AWS_ENDPOINT_URL_BEDROCK: {VERSA_BEDROCK_DEFAULT_ENDPOINT}\n\ + AWS_REGION: {VERSA_BEDROCK_DEFAULT_REGION}\n" + ); + let observed = run_child( + "versas_persisted_endpoint_never_becomes_the_public_providers", + SCENARIO, + &config_yaml, + &[ + ("AWS_ACCESS_KEY_ID", PUBLIC_ACCESS_KEY), + ("AWS_SECRET_ACCESS_KEY", PUBLIC_SECRET_KEY), + ], + ); + let sent = &observed.sent; + assert_eq!( + sent.host, "bedrock-runtime.us-west-2.amazonaws.com", + "Versa's persisted endpoint became the public provider's: {sent:?}" + ); + assert_eq!( + sent.signed_by(), + Some((PUBLIC_ACCESS_KEY, "us-west-2")), + "{sent:?}" + ); +} + +/// …while the public provider still follows the AWS SDK's own variable for +/// this service, which is how a VPC endpoint or a proxy is meant to be set. +#[tokio::test] +async fn the_public_provider_still_follows_the_sdks_endpoint_variable() { + const SCENARIO: &str = "public-with-its-own-endpoint"; + if child_scenario().as_deref() == Some(SCENARIO) { + report(Observed { + resolved: None, + sent: public_sent(public_bound().await).await, + }); + return; + } + + let vpc_endpoint = + "https://vpce-0123456789abcdef0-abcdefgh.bedrock-runtime.us-west-2.vpce.amazonaws.com"; + let observed = run_child( + "the_public_provider_still_follows_the_sdks_endpoint_variable", + SCENARIO, + "AWS_REGION: us-west-2\n", + &[ + ("AWS_ACCESS_KEY_ID", PUBLIC_ACCESS_KEY), + ("AWS_SECRET_ACCESS_KEY", PUBLIC_SECRET_KEY), + ("AWS_ENDPOINT_URL_BEDROCK_RUNTIME", vpc_endpoint), + ], + ); + assert_eq!( + observed.sent.host, + url::Url::parse(vpc_endpoint).unwrap().host_str().unwrap(), + "{:?}", + observed.sent + ); +} + +async fn public_bound() -> BedrockProvider { + crate::config::with_config_overrides( + HashMap::from([("BEDROCK_MAX_RETRIES".into(), "0".into())]), + BedrockProvider::from_env(ModelConfig::new_or_fail(BEDROCK_DEFAULT_MODEL)), + ) + .await + .unwrap_or_else(|e| panic!("the public provider must construct from env credentials: {e}")) +} + +// ------------------------------------------------------------ child process + +/// Names the scenario a re-executed copy of this binary is to run. A +/// test-private key: no production reader resolves it. +const CHILD: &str = "BIOROUTER_TEST_BEDROCK_NAMESPACE_CHILD"; +const REPORT: &str = "BEDROCK_NAMESPACE_OBSERVED "; + +fn child_scenario() -> Option { + std::env::var(CHILD).ok() +} + +fn report(observed: Observed) { + println!("{REPORT}{}", serde_json::to_string(&observed).unwrap()); +} + +/// Re-run `test`, a test in this module, as a child process whose half of the +/// test runs `scenario`. It starts with `env` and with none of the AWS, Versa or +/// Bedrock settings this process inherited, over a config root of its own that +/// holds `config_yaml`, with no AWS profile files and no instance metadata. +fn run_child(test: &str, scenario: &str, config_yaml: &str, env: &[(&str, &str)]) -> Observed { + // A child that reached a parent half would spawn its own child, and so on: + // stop at the first one rather than fork without end. + assert!( + child_scenario().is_none(), + "the child half of `{test}` did not claim scenario `{scenario}`" + ); + let root = tempfile::tempdir().unwrap(); + let config_dir = root.path().join("config"); + std::fs::create_dir_all(&config_dir).unwrap(); + std::fs::write(config_dir.join("config.yaml"), config_yaml).unwrap(); + + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + command.args([ + "--exact", + "--nocapture", + &format!("providers::bedrock_namespace_tests::{test}"), + ]); + for (name, _) in std::env::vars_os() { + let name = name.to_string_lossy(); + if ["AWS_", "VERSA_", "BEDROCK_"] + .iter() + .any(|prefix| name.starts_with(prefix)) + { + command.env_remove(name.as_ref()); + } + } + let output = command + .env(CHILD, scenario) + .env("BIOROUTER_PATH_ROOT", root.path()) + .env("BIOROUTER_DISABLE_KEYRING", "true") + .env("AWS_CONFIG_FILE", root.path().join("aws-config")) + .env( + "AWS_SHARED_CREDENTIALS_FILE", + root.path().join("aws-credentials"), + ) + .env("AWS_EC2_METADATA_DISABLED", "true") + .envs(env.iter().copied()) + .output() + .unwrap(); + + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + let line = stdout + .lines() + .find_map(|line| line.strip_prefix(REPORT)) + .unwrap_or_else(|| { + panic!( + "the child half of `{test}` reported nothing.\n\ + --- child stdout ---\n{stdout}\n--- child stderr ---\n{stderr}" + ) + }); + assert!( + output.status.success(), + "the child half of `{test}` failed.\n--- child stdout ---\n{stdout}\n\ + --- child stderr ---\n{stderr}" + ); + serde_json::from_str(line).unwrap() +} diff --git a/crates/biorouter/src/providers/mod.rs b/crates/biorouter/src/providers/mod.rs index b22889262..f978c5aea 100644 --- a/crates/biorouter/src/providers/mod.rs +++ b/crates/biorouter/src/providers/mod.rs @@ -8,6 +8,8 @@ pub mod azureauth; pub mod base; #[cfg(feature = "aws-providers")] pub mod bedrock; +#[cfg(all(test, feature = "aws-providers"))] +mod bedrock_namespace_tests; pub mod canonical; pub mod claude_code; pub mod codex; @@ -132,11 +134,11 @@ pub(crate) fn is_loopback_host(url: &str) -> bool { /// The tier of a provider that reaches the UCSF gateway and nothing else. /// -/// Demotion only, never promotion: `versa_azure` shares all three -/// `AZURE_OPENAI_*` keys with the public `azure_openai` provider, and -/// `bedrock.rs` sets `AWS_ENDPOINT_URL_BEDROCK_RUNTIME` process-globally, so an -/// endpoint that is not the gateway means the transcript is going somewhere -/// this build cannot vouch for. +/// Demotion only, never promotion: each Versa provider's endpoint is +/// user-writable config (`VERSA_AZURE_ENDPOINT`, `VERSA_BEDROCK_ENDPOINT`), and +/// until 2026-09-11 each also read the public card's keys, so an endpoint that +/// is not the gateway means the transcript is going somewhere this build cannot +/// vouch for. pub(crate) fn ucsf_gateway_tier(endpoint: &str) -> ProviderTier { if host_of(endpoint).as_deref() == Some(UCSF_GATEWAY_HOST) { ProviderTier::Private @@ -164,10 +166,8 @@ pub(crate) fn self_hosted_tier(base_url: &str) -> ProviderTier { /// ones a test thought to list. A name-keyed table (`versa_* => ucsf`) would /// keep claiming the institution for a Versa module repointed at another host, /// which `tier()` had already demoted to Public: a private-looking badge on a -/// public flow. The three `AZURE_OPENAI_*` keys are shared with the public -/// `azure_openai` provider and `bedrock.rs` sets -/// `AWS_ENDPOINT_URL_BEDROCK_RUNTIME` process-globally, so that repointing is a -/// config edit away. +/// public flow. Each Versa endpoint is user-writable config, so that repointing +/// is a config edit away. pub(crate) fn ucsf_gateway_affiliation(endpoint: &str) -> Option { match ucsf_gateway_tier(endpoint) { ProviderTier::Private => Some(*UCSF_AFFILIATION), diff --git a/crates/biorouter/src/providers/tier_tests.rs b/crates/biorouter/src/providers/tier_tests.rs index ebab8c16e..0aae62eef 100644 --- a/crates/biorouter/src/providers/tier_tests.rs +++ b/crates/biorouter/src/providers/tier_tests.rs @@ -291,12 +291,11 @@ fn only_a_loopback_host_reads_as_this_machine() { #[test] fn versa_demotes_when_its_endpoint_is_not_the_ucsf_gateway() { use crate::privacy::ProviderTier::{Private, Public}; - // versa_azure's endpoint is user-writable config (VERSA_AZURE_ENDPOINT; - // until 2026-09-11 also the public azure_openai card's - // AZURE_OPENAI_ENDPOINT), and versa_bedrock falls back to - // AWS_ENDPOINT_URL_BEDROCK_RUNTIME, which bedrock.rs sets PROCESS-GLOBALLY - // with std::env::set_var. The shipped constants are asserted rather than - // their text, so moving a default off the gateway fails here too. + // Both Versa endpoints are user-writable config (VERSA_AZURE_ENDPOINT, + // VERSA_BEDROCK_ENDPOINT), and until 2026-09-11 each also read the public + // card's keys: AZURE_OPENAI_ENDPOINT, and AWS_ENDPOINT_URL_BEDROCK plus the + // process environment. The shipped constants are asserted rather than their + // text, so moving a default off the gateway fails here too. assert_eq!(versa_tier_for_endpoint(VERSA_AZURE_ENDPOINT), Private); assert_eq!( versa_tier_for_endpoint("https://unified-api.ucsf.edu/general"), diff --git a/crates/biorouter/src/providers/versa_bedrock.rs b/crates/biorouter/src/providers/versa_bedrock.rs index 3fbf003d9..0a78d9c76 100644 --- a/crates/biorouter/src/providers/versa_bedrock.rs +++ b/crates/biorouter/src/providers/versa_bedrock.rs @@ -80,9 +80,8 @@ pub struct VersaBedrockProvider { #[serde(skip)] name: String, /// The endpoint this instance resolved at construction. `tier()` reads it, - /// never the provider's name — the last fallback in the chain below is - /// `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`, which `bedrock.rs` sets - /// process-globally with `std::env::set_var`. + /// never the provider's name — `VERSA_BEDROCK_ENDPOINT` is user-writable, so + /// an instance can resolve somewhere that is not the UCSF gateway. #[serde(skip)] resolved_endpoint: String, #[serde(skip)] @@ -95,33 +94,42 @@ impl VersaBedrockProvider { pub async fn from_env(model: ModelConfig) -> Result { let config = crate::config::Config::global(); - // Endpoint: configurable, but always falls back to the UCSF MuleSoft proxy - // so a fresh install with just the key + secret works out of the box. + // Overrides come from this provider's OWN namespace and nowhere else. A + // blank value is absent, and absent is the UCSF gateway, so a fresh + // install with just the key and secret works out of the box. + // + // ⚠ Not `AWS_ENDPOINT_URL_BEDROCK` or `AWS_REGION`, and not the process + // environment. The `AWS_*` namespace is the public `aws_bedrock` card's: + // it declares `AWS_REGION`, and `bedrock.rs` exports every `AWS_*` + // config value and secret into the environment. Sharing it went wrong in + // both directions. Versa read those two keys, then + // `AWS_ENDPOINT_URL_BEDROCK` and `AWS_ENDPOINT_URL_BEDROCK_RUNTIME` from + // the environment, as fallbacks. So the public card's region, or an + // endpoint left by its export or by a shell, steered Versa: UCSF-issued + // keys signed requests for someone's own AWS region, which refused them, + // and the instance turned Public. Versa's setup also WROTE both keys, so + // connecting it marked the public card Configured, and handed it UCSF's + // gateway as an endpoint. The fallbacks are gone (2026-09-11). + // + // Where nothing was set, every Versa setup surface prefilled the shipped + // defaults, and neither default has changed: the region has been + // us-west-2 since Versa Bedrock shipped (2026-05-07), and the endpoint + // has been UCSF's gateway since it became configurable (2026-05-12). So a + // value this drops was either typed over that prefill or came from the + // public side, and the second is the bug itself. + // + // ⚠ Each key is a STRING LITERAL passed straight to `get_param`, as in + // `versa_azure`, because `privacy::config_keys` scans this file for them. let endpoint_url: String = config - .get_param::("AWS_ENDPOINT_URL_BEDROCK") + .get_param::("VERSA_BEDROCK_ENDPOINT") .ok() .filter(|s| !s.trim().is_empty()) - .or_else(|| { - std::env::var("AWS_ENDPOINT_URL_BEDROCK") - .ok() - .filter(|s| !s.trim().is_empty()) - }) - .or_else(|| { - std::env::var("AWS_ENDPOINT_URL_BEDROCK_RUNTIME") - .ok() - .filter(|s| !s.trim().is_empty()) - }) .unwrap_or_else(|| VERSA_BEDROCK_DEFAULT_ENDPOINT.to_string()); let region: String = config - .get_param::("AWS_REGION") + .get_param::("VERSA_BEDROCK_REGION") .ok() .filter(|s| !s.trim().is_empty()) - .or_else(|| { - std::env::var("AWS_REGION") - .ok() - .filter(|s| !s.trim().is_empty()) - }) .unwrap_or_else(|| VERSA_BEDROCK_DEFAULT_REGION.to_string()); let retry_config = Self::load_retry_config(config); @@ -198,6 +206,17 @@ impl VersaBedrockProvider { Credentials::new(access_key_id, secret_access_key, None, None, "VersaBedrock"); let loader = aws_config::defaults(aws_config::BehaviorVersion::latest()) .credentials_provider(credentials) + // ⚠ SigV4 with the credentials above, chosen in code. The AWS SDK + // reads `AWS_BEARER_TOKEN_BEDROCK` from the process environment by + // itself, and unless the auth scheme was chosen in code it then + // authenticates with that bearer token instead of signing. That + // variable is where AWS tells a user to put a Bedrock API key, i.e. + // the PUBLIC card's credential. Without this line a Versa chat that + // looked entirely right (UCSF gateway, us-west-2, Private) sent the + // public card's API key to UCSF, and Versa's own keys signed + // nothing. A preference set on this loader counts as chosen in code + // (`Origin::is_client_config`), so the SDK leaves it alone. + .auth_scheme_preference(["sigv4".into()]) .region(aws_config::Region::new(region.clone())) .endpoint_url(endpoint.as_str()); #[cfg(test)] @@ -238,6 +257,20 @@ impl VersaBedrockProvider { }) } + /// The same client with only its HTTP transport replaced. Everything the + /// constructor resolved — endpoint, region, credentials, auth scheme — is + /// kept, so a request captured through it is the request production would + /// have sent. + #[cfg(test)] + pub(crate) fn with_http_client( + mut self, + http_client: impl aws_sdk_bedrockruntime::config::HttpClient + 'static, + ) -> Self { + let config = self.client.config().to_builder().http_client(http_client); + self.client = Client::from_conf(config.build()); + self + } + fn load_retry_config(config: &crate::config::Config) -> RetryConfig { let max_retries = config .get_param::("BEDROCK_MAX_RETRIES") @@ -367,21 +400,24 @@ impl Provider for VersaBedrockProvider { VERSA_BEDROCK_DEFAULT_MODEL, models, VERSA_BEDROCK_DOC_LINK, + // ⚠ The key and secret, and NOTHING else, as the description above + // says. This used to declare `AWS_ENDPOINT_URL_BEDROCK` and + // `AWS_REGION`, and the setup form persists a declared key's default + // (DefaultProviderSetupForm seeds it as a value; DefaultSubmitHandler + // submits it). `AWS_REGION` is one of the two keys the PUBLIC + // `aws_bedrock` card declares, both required and both defaulted, so + // `check_provider_configured` calls that card Configured once either + // is in `config.yaml`: setting up UCSF's private Versa lit up the + // public, commercial Amazon Bedrock card. `versa_azure` had the same + // defect with the public Azure card (2026-09-03). + // + // Dropping them costs nothing. An install that sets nothing still + // reaches the UCSF gateway through the constants above, and an + // operator overrides through Versa's own `VERSA_BEDROCK_*` keys (see + // `from_env`). vec![ ConfigKey::new("VERSA_BEDROCK_ACCESS_KEY_ID", true, true, None), ConfigKey::new("VERSA_BEDROCK_SECRET_ACCESS_KEY", true, true, None), - ConfigKey::new( - "AWS_ENDPOINT_URL_BEDROCK", - false, - false, - Some(VERSA_BEDROCK_DEFAULT_ENDPOINT), - ), - ConfigKey::new( - "AWS_REGION", - false, - false, - Some(VERSA_BEDROCK_DEFAULT_REGION), - ), ], ) .with_unlisted_models() @@ -752,10 +788,9 @@ mod tests { /// `tier_tests.rs`, but a test of the predicate alone cannot see whether /// this provider calls it, or hands it the right field. Replace the body of /// `tier()` with an unconditional `Private` and every one of those tests - /// still passes. This one does not — and the demotion matters most here, - /// because the last fallback in `from_env`'s endpoint chain is - /// `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`, which `bedrock.rs` sets - /// **process-globally** with `std::env::set_var`. + /// still passes. This one does not. The demotion is still needed although + /// `from_env` no longer falls back to the public side's keys: + /// `VERSA_BEDROCK_ENDPOINT` is user-writable config. #[tokio::test] async fn tier_follows_the_endpoint_this_instance_resolved() { let shipped = provider_at(VERSA_BEDROCK_DEFAULT_ENDPOINT).await; @@ -773,12 +808,9 @@ mod tests { } /// DR-26 (Task 46) rule, **wired** — the same argument as the tier test - /// above, for the third axis, and it matters most here: the last fallback in - /// `from_env`'s endpoint chain is `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`, which - /// `bedrock.rs` sets **process-globally** with `std::env::set_var`. An - /// affiliation keyed on the provider's name would keep claiming `ucsf` for - /// an instance that another provider's construction had already repointed at - /// a plain AWS region. + /// above, for the third axis. `VERSA_BEDROCK_ENDPOINT` is user-writable, and + /// an affiliation keyed on the provider's name would keep claiming `ucsf` + /// for an instance repointed at a plain AWS region. #[tokio::test] async fn affiliation_follows_the_endpoint_this_instance_resolved() { use crate::privacy::affiliation::{InstitutionId, ModelAffiliation}; diff --git a/crates/biorouter/tests/versa_stream_wire_probe.rs b/crates/biorouter/tests/versa_stream_wire_probe.rs index d7490cc7a..f6d403e63 100644 --- a/crates/biorouter/tests/versa_stream_wire_probe.rs +++ b/crates/biorouter/tests/versa_stream_wire_probe.rs @@ -145,11 +145,12 @@ fn transport_http1_only(mode: &str) -> ProbeResult { impl Settings { fn load() -> ProbeResult { let config = Config::global(); - let endpoint = configured_string(config, "AWS_ENDPOINT_URL_BEDROCK") - .or_else(|| nonempty_env("AWS_ENDPOINT_URL_BEDROCK_RUNTIME")) + // Versa's own keys only, as `VersaBedrockProvider::from_env` reads them: + // the `AWS_*` ones belong to the public Amazon Bedrock card. + let endpoint = configured_string(config, "VERSA_BEDROCK_ENDPOINT") .unwrap_or_else(|| VERSA_BEDROCK_DEFAULT_ENDPOINT.into()); validate_endpoint(&endpoint)?; - let region = configured_string(config, "AWS_REGION") + let region = configured_string(config, "VERSA_BEDROCK_REGION") .unwrap_or_else(|| VERSA_BEDROCK_DEFAULT_REGION.into()); if region.len() > 32 || !region diff --git a/docs/security/privacy-tiers.md b/docs/security/privacy-tiers.md index dc3e9437a..853be5f09 100644 --- a/docs/security/privacy-tiers.md +++ b/docs/security/privacy-tiers.md @@ -681,6 +681,16 @@ already resolved. > `_DEPLOYMENT_NAME` / `_API_VERSION`, so the shared-key half of this hazard is closed at the source: > whatever the public `azure_openai` card is set up with no longer reaches it. The demotion rule is > unchanged and still needed, because `VERSA_AZURE_ENDPOINT` is user-writable config. +> +> **Update (2026-09-11), Bedrock.** `versa_bedrock` now reads only its own `VERSA_BEDROCK_ENDPOINT` / +> `VERSA_BEDROCK_REGION`, with no fallback to an `AWS_*` key or to the process environment, and +> `bedrock.rs` no longer promotes `AWS_ENDPOINT_URL_BEDROCK` to `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`. +> That closes the fallback named above in both directions. The shared namespace also hid a crossing +> no endpoint check can see: the AWS SDK reads `AWS_BEARER_TOKEN_BEDROCK` from the environment itself +> and authenticated Versa's requests with that token. So an instance that resolved the gateway, and +> was rightly Private, carried the public card's Bedrock API key to UCSF. `versa_bedrock` now chooses +> SigV4 in code. The demotion rule is unchanged and still needed, because `VERSA_BEDROCK_ENDPOINT` +> is user-writable config. **Never keyed on a model id.** `us.anthropic.claude-opus-4-8` appears in both `BEDROCK_KNOWN_MODELS` and `VERSA_BEDROCK_KNOWN_MODELS`. Any model-name badge is wrong by diff --git a/docs/testing/process-global-state.md b/docs/testing/process-global-state.md index 1338ed11a..2dabb0607 100644 --- a/docs/testing/process-global-state.md +++ b/docs/testing/process-global-state.md @@ -69,18 +69,19 @@ The four highest-traffic literal keys are `BIOROUTER_PATH_ROOT` (6 sites), `PATH ### Production code that writes the environment -Five sites, and they are not a test concern — they mutate the environment of a multi-threaded daemon. +Four sites, and they are not a test concern — they mutate the environment of a multi-threaded daemon. | Site | What it writes | |---|---| | `providers/bedrock.rs:79` | every `AWS_*` config value **and secret**, from `from_env` | -| `providers/bedrock.rs:90-92` | an unlocked read-then-write promoting `AWS_ENDPOINT_URL_BEDROCK` | | `providers/sagemaker_tgi.rs:52` | the same `AWS_*` dump | | `config/base.rs:1504` | `BIOROUTER_DISABLE_KEYRING=1` on keyring fallback; races `Config::default`'s read at `:225`, which `GLOBAL_CONFIG` then freezes | | `agents/test_sandbox.rs:37` | safe twice over — a `#[ctor]` that runs before `main`, in a module gated at its declaration site | The two `AWS_*` dumps leak credentials into every subprocess spawned afterwards, which is the exact unsoundness the `CONFIG_OVERRIDES` task-local was introduced to avoid. +A fifth site, an unlocked read-then-write at `providers/bedrock.rs:90-92` that promoted `AWS_ENDPOINT_URL_BEDROCK` to `AWS_ENDPOINT_URL_BEDROCK_RUNTIME`, was removed on 2026-09-11. It is how the UCSF gateway that Versa Bedrock's setup persisted became the public `aws_bedrock` provider's endpoint; see `providers/bedrock_namespace_tests.rs`. + ### Test writers | Shape | Count | @@ -130,6 +131,7 @@ Verdicts: **fixed**, **live** (a reader can observe another test's write today), | `pending_user_action::USER_PROOF_AVAILABLE` | 6 lib readers | none in `--lib` | **latent** | | `SkillsClient::new` resolving `Paths::config_dir()` | `agents/skills_extension.rs:807` | — | **accepted** — PR #193 calls the synchronous constructor read "the property we want": the root a client seeds into is the one that was ambient when it was built | | `AWS_*` written by production | any `env::var` reader in the process | `providers/bedrock.rs:79`, `sagemaker_tgi.rs:52` | **open** — not a test hazard; recorded here because it is the same mechanism | +| `AWS_BEARER_TOKEN_BEDROCK` | the AWS SDK itself: Bedrock Runtime's `From<&SdkConfig>` reads it and prefers bearer auth unless the auth scheme was chosen in code, reached from `VersaBedrockProvider::from_resolved` | a shell, or `providers/bedrock.rs:79`'s export | **fixed** 2026-09-11 — Versa's loader chooses SigV4 in code. A reader inside a dependency is invisible to every `env::var` scan in this document. | ### `/skills`, spelled eleven ways diff --git a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx index e473575fa..847a9befc 100644 --- a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx +++ b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx @@ -27,6 +27,17 @@ async function connectVersaAzure() { await waitFor(() => expect(mockCheckProvider).toHaveBeenCalled()); } +async function connectVersaBedrock() { + const onSuccess = vi.fn(); + render(); + fireEvent.click(screen.getByRole('tab', { name: /Bedrock/i })); + fireEvent.change(screen.getByLabelText(/Access Key ID/i), { target: { value: 'an-id' } }); + fireEvent.change(screen.getByLabelText(/Secret Access Key/i), { target: { value: 'a-secret' } }); + fireEvent.click(screen.getByRole('button', { name: /Connect to Versa Bedrock/i })); + // Past `checkProvider`, so every write the connect makes has been recorded. + await waitFor(() => expect(onSuccess).toHaveBeenCalledWith('versa_bedrock')); +} + describe('InstitutionalSetupCard', () => { beforeEach(() => { vi.clearAllMocks(); @@ -55,4 +66,27 @@ describe('InstitutionalSetupCard', () => { expect(written).toContain('VERSA_AZURE_DEPLOYMENT_NAME'); expect(written).toContain('VERSA_AZURE_API_VERSION'); }); + + it('never writes a key in the public AWS namespace when connecting UCSF Versa Bedrock', async () => { + // Connecting UCSF's PRIVATE Versa Bedrock used to write `AWS_REGION` and + // `AWS_ENDPOINT_URL_BEDROCK`. The public `aws_bedrock` card declares + // `AWS_REGION`, so the write marked that card Configured and replaced its + // region; and `bedrock.rs` exports every `AWS_*` key into the process + // environment, which is how the UCSF gateway became the public provider's + // endpoint. + await connectVersaBedrock(); + const written = mockUpsert.mock.calls.map((c) => c[0] as string); + expect(written.filter((key) => key.startsWith('AWS_'))).toEqual([]); + }); + + it('writes the Versa Bedrock credentials and overrides, then selects the provider', async () => { + await connectVersaBedrock(); + expect(mockUpsert.mock.calls).toEqual([ + ['VERSA_BEDROCK_ACCESS_KEY_ID', 'an-id', true], + ['VERSA_BEDROCK_SECRET_ACCESS_KEY', 'a-secret', true], + ['VERSA_BEDROCK_ENDPOINT', 'https://unified-api.ucsf.edu/general/awsai', false], + ['VERSA_BEDROCK_REGION', 'us-west-2', false], + ['BIOROUTER_PROVIDER', 'versa_bedrock', false], + ]); + }); }); diff --git a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.tsx b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.tsx index 3a6739f9f..a297f40be 100644 --- a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.tsx +++ b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.tsx @@ -15,9 +15,13 @@ interface InstitutionalSetupCardProps { type VersaFlavor = 'azure' | 'bedrock'; +// Versa Bedrock's own keys, the only ones `versa_bedrock.rs` reads. This card +// used to write `AWS_ENDPOINT_URL_BEDROCK` and `AWS_REGION`, which belong to the +// PUBLIC Amazon Bedrock card: the write marked that card Configured and replaced +// its region, and the public provider took UCSF's gateway as its endpoint. const VERSA_BEDROCK_DEFAULTS = { - AWS_ENDPOINT_URL_BEDROCK: 'https://unified-api.ucsf.edu/general/awsai', - AWS_REGION: 'us-west-2', + VERSA_BEDROCK_ENDPOINT: 'https://unified-api.ucsf.edu/general/awsai', + VERSA_BEDROCK_REGION: 'us-west-2', }; const VERSA_AZURE_DEFAULTS = { @@ -61,9 +65,9 @@ export default function InstitutionalSetupCard({ const [bedrockSecretKey, setBedrockSecretKey] = useState(''); const [azureApiKey, setAzureApiKey] = useState(''); const [bedrockEndpoint, setBedrockEndpoint] = useState( - VERSA_BEDROCK_DEFAULTS.AWS_ENDPOINT_URL_BEDROCK + VERSA_BEDROCK_DEFAULTS.VERSA_BEDROCK_ENDPOINT ); - const [bedrockRegion, setBedrockRegion] = useState(VERSA_BEDROCK_DEFAULTS.AWS_REGION); + const [bedrockRegion, setBedrockRegion] = useState(VERSA_BEDROCK_DEFAULTS.VERSA_BEDROCK_REGION); const [azureEndpoint, setAzureEndpoint] = useState(VERSA_AZURE_DEFAULTS.VERSA_AZURE_ENDPOINT); const [azureDeployment, setAzureDeployment] = useState( VERSA_AZURE_DEFAULTS.VERSA_AZURE_DEPLOYMENT_NAME @@ -91,8 +95,8 @@ export default function InstitutionalSetupCard({ if (flavor === 'bedrock') { await upsert('VERSA_BEDROCK_ACCESS_KEY_ID', bedrockAccessKey.trim(), true); await upsert('VERSA_BEDROCK_SECRET_ACCESS_KEY', bedrockSecretKey.trim(), true); - await upsert('AWS_ENDPOINT_URL_BEDROCK', bedrockEndpoint.trim(), false); - await upsert('AWS_REGION', bedrockRegion.trim(), false); + await upsert('VERSA_BEDROCK_ENDPOINT', bedrockEndpoint.trim(), false); + await upsert('VERSA_BEDROCK_REGION', bedrockRegion.trim(), false); await checkProvider({ body: { provider: 'versa_bedrock' }, throwOnError: true }); await upsert('BIOROUTER_PROVIDER', 'versa_bedrock', false); onSuccess('versa_bedrock'); @@ -263,7 +267,7 @@ export default function InstitutionalSetupCard({ <>
- + > = { AZURE_OPENAI_DEPLOYMENT_NAME: 'gpt-5.5-2026-04-24', AZURE_OPENAI_API_VERSION: '2025-01-01-preview', }, - versa_bedrock: { - AWS_ENDPOINT_URL_BEDROCK: 'https://unified-api.ucsf.edu/general/awsai', - AWS_REGION: 'us-west-2', - }, }; const envToPrettyName = (envVar: string) => {