From 967b2149fb79e8e16b76c4a5ee0bac94555e9354 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:39:48 -0700 Subject: [PATCH 1/9] fix(versa_azure): the public Azure card's deployment no longer routes Versa requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `VersaAzureProvider::from_env` fell back to `AZURE_OPENAI_DEPLOYMENT_NAME` whenever `VERSA_AZURE_DEPLOYMENT_NAME` was blank. That key is not Versa's: it is the one key the PUBLIC `azure_openai` card requires and ships no default for, so it names a deployment on whatever Azure resource the user set THAT card up with. A name the catalog does not know is an override serving every Versa request, so a user who configured Azure OpenAI with `my-gpt4o` (or `gpt-4o`, Azure's habit of naming a deployment after its model) had every Versa turn posted to https://unified-api.ucsf.edu/general/openai/deployments/my-gpt4o/... and got DeploymentNotFound back, whatever model the chat named. Where the name is a real UCSF deployment the catalog does not offer (gpt-5-mini-2025-08-07 answers at the gateway), the wrong model replied and nothing said so: F1 again, through the other provider's card. This predates #230 — the fallback used to pin the deployment outright; #230 narrowed it to non-catalog names. Reproduced before fixing: the new routing test, run against faf52596, fails with left: "/openai/deployments/my-gpt4o/chat/completions" right: "/openai/deployments/gpt-4.1-2025-04-14/chat/completions" Exposed: every install whose Versa was not set up through the onboarding card after 2026-09-03, the only surface that writes VERSA_AZURE_DEPLOYMENT_NAME. The Settings form and `biorouter configure` write only the declared API key. The fix: read Versa's own key and nothing else. Migration. The fallback was kept for installs whose pre-2026-09-03 Versa forms wrote the legacy key. Those forms PREFILLED the shipped default (gpt-5.2-2025-12-11 until 2026-07-02, gpt-5.5-2026-04-24 since), and both name catalog deployments, which are not overrides — for those installs nothing changes. What is dropped is a value someone typed over the prefill in the onboarding card's Advanced panel or the Settings form's collapsed options: that install's chats now post to each model's own deployment, a model the catalog does not map is refused before anything is sent, and setting VERSA_AZURE_DEPLOYMENT_NAME restores the override. A session row written while the fallback was live may store the public card's deployment as its route; nothing in the row tells it from a chosen override, so a restore honours it until the chat's model is picked again, which rebuilds through `from_env`. Privacy key scan. Of the five tier-input files only versa_azure.rs read AZURE_OPENAI_DEPLOYMENT_NAME (factory.rs never did; azure.rs is not scanned), so the scanned surface drops 26 -> 25 and the key's "deployment selection" row is deleted rather than left classifying nothing. The test now also asserts the reverse direction — every classified key is still read by a tier-input file — so the next removed read cannot be absorbed by editing the count alone. The one assertion of the old behaviour ("the legacy key still overrides where Versa's own is unset") is replaced by its opposite: Versa's own key wins, and the public key beside it changes nothing. The server test that pinned the legacy key to isolate a private first bind now pins VERSA_AZURE_DEPLOYMENT_NAME, and the manual stream probe requires it. --- crates/biorouter-server/src/routes/agent.rs | 2 +- crates/biorouter/src/privacy/config_keys.rs | 29 +++- crates/biorouter/src/providers/versa_azure.rs | 141 +++++++++++++----- .../biorouter/tests/versa_gpt_stream_probe.rs | 4 +- 4 files changed, 134 insertions(+), 42 deletions(-) diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index 1800fa818..7c7e0efd4 100644 --- a/crates/biorouter-server/src/routes/agent.rs +++ b/crates/biorouter-server/src/routes/agent.rs @@ -3061,7 +3061,7 @@ mod new_session_provider_binding_tests { biorouter::providers::versa_azure::VERSA_AZURE_ENDPOINT.into(), ); overrides.insert( - "AZURE_OPENAI_DEPLOYMENT_NAME".into(), + "VERSA_AZURE_DEPLOYMENT_NAME".into(), biorouter::providers::versa_azure::deployment_for_model( biorouter::providers::versa_azure::VERSA_AZURE_DEFAULT_MODEL, ) diff --git a/crates/biorouter/src/privacy/config_keys.rs b/crates/biorouter/src/privacy/config_keys.rs index 814599443..3b48d608e 100644 --- a/crates/biorouter/src/privacy/config_keys.rs +++ b/crates/biorouter/src/privacy/config_keys.rs @@ -65,13 +65,17 @@ pub const NOT_CAPABILITY_CONFIG_KEYS: &[(&str, &str)] = &[ "AZURE_OPENAI_ENDPOINT", "moves a Private provider's endpoint; does not raise a tier (see Task 5)", ), - ("AZURE_OPENAI_DEPLOYMENT_NAME", "deployment selection"), + // No `AZURE_OPENAI_DEPLOYMENT_NAME` row: no tier-input file reads it. It is + // the public `azure_openai` card's required key, and `versa_azure` reading + // it as a fallback let that card's deployment route every Versa request, so + // the read was removed (2026-09-11). `azure.rs` still reads it and is not a + // tier-input file: `azure_openai` is Public whatever deployment it names. ("AZURE_OPENAI_API_VERSION", "wire version"), // Versa's own namespace for the same three overrides. They exist because // onboarding used to write the `AZURE_OPENAI_*` keys above on Versa's // behalf, which made the PUBLIC `azure_openai` card report itself - // Configured whenever a user connected UCSF's PRIVATE Versa. Same meaning, - // same classification as their legacy twins: they move a Private + // Configured whenever a user connected UCSF's PRIVATE Versa. Same + // classification as the legacy reads they replace: they move a Private // provider's endpoint, they do not raise a tier. ( "VERSA_AZURE_ENDPOINT", @@ -183,10 +187,10 @@ mod tests { // of the two lists. Adding a config read to any of them fails this test // until someone decides whether it determines capability. That is the // checkable list: it does not depend on anyone remembering a rule. - let scanned = scan_get_param_keys(); // 26 today + let scanned = scan_get_param_keys(); // 25 today assert_eq!( scanned.len(), - 26, + 25, "the tier-input files' config surface changed: {scanned:?}" ); for key in &scanned { @@ -205,6 +209,21 @@ mod tests { // survives. assert!(CAPABILITY_CONFIG_KEYS.contains(&"BIOROUTER_PROVIDER")); assert_eq!(CAPABILITY_CONFIG_KEYS.len(), 5); + + // …and the other way round: every classified key is still READ by a + // tier-input file. Without this, a read that goes away leaves its row + // behind — the count above moves, someone edits the number, and the + // lists quietly start classifying keys nothing reads. + let classified = CAPABILITY_CONFIG_KEYS + .iter() + .copied() + .chain(NOT_CAPABILITY_CONFIG_KEYS.iter().map(|(key, _why)| *key)); + for key in classified.filter(|key| *key != "BIOROUTER_PROVIDER") { + assert!( + scanned.iter().any(|read| read == key), + "{key} is classified but no tier-input file reads it; delete its row" + ); + } } #[test] diff --git a/crates/biorouter/src/providers/versa_azure.rs b/crates/biorouter/src/providers/versa_azure.rs index bd8af9ef7..bb5893674 100644 --- a/crates/biorouter/src/providers/versa_azure.rs +++ b/crates/biorouter/src/providers/versa_azure.rs @@ -100,19 +100,17 @@ const NO_DEPLOYMENT_ROUTE: &str = /// The deployment override `value` amounts to, if any. /// -/// Judges BOTH sources of one — a configured `VERSA_AZURE_DEPLOYMENT_NAME` (or -/// its legacy twin) and the `deployment` a session's restore binding stored — -/// so a live provider and a restored one cannot disagree about whether an -/// override is in force. +/// Judges BOTH sources of one — a configured `VERSA_AZURE_DEPLOYMENT_NAME` and +/// the `deployment` a session's restore binding stored — so a live provider and +/// a restored one cannot disagree about whether an override is in force. /// /// ⚠ A value that names a CATALOG deployment is not an override, and the fix /// turns on it: /// /// * Nobody chose it. The onboarding card upserts `VERSA_AZURE_DEPLOYMENT_NAME` -/// with the shipped default on every connect, and before 2026-09-03 the -/// setup form wrote `AZURE_OPENAI_DEPLOYMENT_NAME` the same way. Honouring -/// either would pin every onboarded install to one model — F1 again, in -/// exactly the installs the QA sandbox did not have. +/// with the shipped default on every connect. Honouring it would pin every +/// onboarded install to one model — F1 again, in exactly the installs the +/// QA sandbox did not have. /// * It is what every stored binding says. Before this change `deployment` /// was the fixed default whatever the model, and a subagent's model override /// still rewrites the binding's model without touching its route @@ -123,6 +121,11 @@ const NO_DEPLOYMENT_ROUTE: &str = /// /// What is left is the real escape hatch: a deployment the catalog does not /// know, which then serves every request. +/// +/// A row written by a build that still read the public `azure_openai` card's +/// `AZURE_OPENAI_DEPLOYMENT_NAME` may carry that card's deployment here, and +/// nothing in the row tells it from a chosen one — so it is honoured like one +/// until the chat's model is picked again, which rebuilds through `from_env`. fn explicit_override(value: Option<&str>) -> Option { let value = value?.trim(); (!value.is_empty() && value != NO_DEPLOYMENT_ROUTE && !is_catalog_deployment(value)) @@ -274,15 +277,22 @@ impl VersaAzureProvider { // What the configuration names is a CANDIDATE override, and // `explicit_override` decides whether it is one — which is what keeps // the default the onboarding card persists from pinning every model. - let configured_deployment = resolve_override( - "", - config - .get_param::("VERSA_AZURE_DEPLOYMENT_NAME") - .ok(), - config - .get_param::("AZURE_OPENAI_DEPLOYMENT_NAME") - .ok(), - ); + // + // ⚠ Versa's OWN key, and no fallback. `AZURE_OPENAI_DEPLOYMENT_NAME` + // used to be read after it, and it is not Versa's to read: it is the + // one key the public `azure_openai` card requires and ships no default + // for, so it names a deployment on whatever Azure resource the user set + // THAT card up with (`my-gpt4o`, or `gpt-4o` — Azure's habit of naming a + // deployment after its model). The catalog knows no such name, so it + // became an override serving every Versa request: DeploymentNotFound on + // each turn, or — where it is a real UCSF deployment the catalog does + // not offer — a silent answer from the wrong model. The fallback was + // kept for installs whose pre-2026-09-03 Versa forms wrote that key, + // and those forms prefilled catalog deployments, which are not + // overrides; only a value someone typed over the prefill is dropped. + let configured_deployment = config + .get_param::("VERSA_AZURE_DEPLOYMENT_NAME") + .unwrap_or_default(); let deployment_override = explicit_override(Some(&configured_deployment)); match &deployment_override { Some(deployment) => tracing::info!( @@ -519,10 +529,10 @@ impl Provider for VersaAzureProvider { // Configured — pointing at the UCSF endpoint while carrying the // weaker Public tier. Reported from the field. // - // Removing them costs nothing: `from_env` reads the same names with - // `unwrap_or_else(|_| VERSA_AZURE_*)`, so an operator who sets them - // in the environment or config still overrides, and an install that - // sets nothing still gets the UCSF gateway. + // Removing them costs nothing: an install that sets nothing still + // gets the UCSF gateway, because `from_env` falls back to the + // `VERSA_AZURE_*` constants, and an operator overrides through + // Versa's own `VERSA_AZURE_*` keys (see `from_env`). vec![ConfigKey::new("VERSA_AZURE_API_KEY", true, true, None)], ) // ⚠ No `with_unlisted_models()`, which this provider used to declare. A @@ -970,12 +980,15 @@ mod tests { ); assert!(!is_catalog_deployment(NO_DEPLOYMENT_ROUTE)); - // Every default a setup surface ever persisted must stay a non-override. + // Every default a setup surface ever persisted must stay a non-override: + // the onboarding card writes gpt-5.5 into `VERSA_AZURE_DEPLOYMENT_NAME`, + // and a session row written by a build that still read the legacy + // `AZURE_OPENAI_DEPLOYMENT_NAME` stored that key's default as its route. // Today each is a catalog deployment, which is the only reason it is // ignored — so trimming one from the catalog would silently turn it into - // an override and pin every install that onboarded while it shipped. If - // this fails after a trim, keep recognising the value (a retired-defaults - // list beside the catalog) rather than editing it out of this test. + // an override and pin every chat that carries it. If this fails after a + // trim, keep recognising the value (a retired-defaults list beside the + // catalog) rather than editing it out of this test. for shipped_default in [ "gpt-5.2-2025-12-11", // provider default 2026-05-07 .. 2026-07-02 "gpt-5.5-2026-04-24", // provider default since; the onboarding card's @@ -1242,10 +1255,11 @@ mod routing_tests { provider } - /// A key, the two deployment keys as given, and every other override key - /// blank — blank is absent — so the machine running the suite cannot leak - /// its own configuration into what is being measured. - fn config(deployment: &str, legacy_deployment: &str) -> HashMap { + /// A key, Versa's deployment key and the public `azure_openai` card's as + /// given, and every other override key blank — blank is absent — so the + /// machine running the suite cannot leak its own configuration into what is + /// being measured. + fn config(deployment: &str, public_deployment: &str) -> HashMap { HashMap::from([ ("VERSA_AZURE_API_KEY".into(), "test-key".into()), ("VERSA_AZURE_ENDPOINT".into(), String::new()), @@ -1255,7 +1269,7 @@ mod routing_tests { ("VERSA_AZURE_DEPLOYMENT_NAME".into(), deployment.into()), ( "AZURE_OPENAI_DEPLOYMENT_NAME".into(), - legacy_deployment.into(), + public_deployment.into(), ), ]) } @@ -1411,6 +1425,64 @@ mod routing_tests { ); } + /// `AZURE_OPENAI_DEPLOYMENT_NAME` belongs to the PUBLIC `azure_openai` + /// provider: it is the one key that card requires and ships no default for, + /// so whoever set it up typed a deployment on THEIR Azure resource into it. + /// Versa used to read it as a fallback, and a name the catalog does not know + /// then served every Versa request. None of these may route one — each + /// fails differently at the real gateway, so each is here. A catalog name in + /// that key was never an override; for those see + /// `a_persisted_default_deployment_does_not_pin_the_model`. + #[tokio::test] + async fn the_public_azure_cards_deployment_never_routes_a_versa_request() { + let server = gateway().await; + let public_deployments = [ + // A company deployment: DeploymentNotFound on every Versa turn. + "my-gpt4o", + // Azure's habit of naming a deployment after its model. The short + // aliases are DeploymentNotFound at the gateway (measured). + "gpt-4o", + // A real UCSF deployment the catalog does not offer: the gateway + // ANSWERS, so the wrong model replies and nothing says so — F1. + "gpt-5-mini-2025-08-07", + ]; + for public_deployment in public_deployments { + let chat = aimed_at( + bound("gpt-4.1-2025-04-14", config("", public_deployment)).await, + &server, + ); + let answered = answered_by(&chat).await.unwrap(); + assert_eq!( + requested_paths(&server).await.pop().unwrap_or_default(), + path_of("gpt-4.1-2025-04-14"), + "the public azure_openai card's deployment `{public_deployment}` routed a \ + Versa request for gpt-4.1-2025-04-14" + ); + assert_eq!(answered, "gpt-4.1-2025-04-14"); + // The route a reopened chat reuses is the model's own too, so the + // other card's value cannot be persisted into the session row. + assert_eq!( + serde_json::to_value(chat.restore_binding()).unwrap()["deployment"], + "gpt-4.1-2025-04-14", + "`{public_deployment}` was written into the restore binding" + ); + + // Nor may it rescue a model no deployment serves. + let probe = aimed_at( + bound(UNMAPPED, config("", public_deployment)).await, + &server, + ); + assert!( + answered_by(&probe).await.is_err(), + "`{public_deployment}` answered for a model no Versa deployment serves" + ); + } + assert_eq!( + requested_paths(&server).await, + vec![path_of("gpt-4.1-2025-04-14"); public_deployments.len()] + ); + } + /// Every row written before this change stores gpt-5.5's deployment, /// whatever its model — the QA run read exactly that off a rebound chat. A /// restore must re-derive the route from the model, not carry it forward. @@ -1451,12 +1523,13 @@ mod routing_tests { &server, ); assert_eq!(answered_by(&live).await.unwrap(), custom); - // The legacy key still overrides where Versa's own is unset. - let legacy = aimed_at( - bound("gpt-4.1-2025-04-14", config("", custom)).await, + // Versa's own key is the one way to set it, and the public card's key + // naming some other deployment does not compete with it. + let beside_public = aimed_at( + bound("gpt-4.1-2025-04-14", config(custom, "my-gpt4o")).await, &server, ); - assert_eq!(answered_by(&legacy).await.unwrap(), custom); + assert_eq!(answered_by(&beside_public).await.unwrap(), custom); // An override is the operator saying where requests go, so it serves a // model the catalog does not list as well. let unlisted = aimed_at(bound(UNMAPPED, config(custom, "")).await, &server); diff --git a/crates/biorouter/tests/versa_gpt_stream_probe.rs b/crates/biorouter/tests/versa_gpt_stream_probe.rs index 48d72109c..9d4b9116d 100644 --- a/crates/biorouter/tests/versa_gpt_stream_probe.rs +++ b/crates/biorouter/tests/versa_gpt_stream_probe.rs @@ -1,6 +1,6 @@ //! Opt-in synthetic provider/decoder probe; returned tools are never executed. //! Requires BIOROUTER_RUN_VERSA_GPT_PROBE=1, BIOROUTER_VERSA_GPT_PROBE_MODEL, -//! matching AZURE_OPENAI_DEPLOYMENT_NAME, AZURE_OPENAI_ENDPOINT, and a NEW +//! matching VERSA_AZURE_DEPLOYMENT_NAME, AZURE_OPENAI_ENDPOINT, and a NEW //! BIOROUTER_VERSA_GPT_PROBE_OUTPUT directly under /tmp. CASE defaults to todo; //! BIOROUTER_VERSA_GPT_PROBE_CASE=python_sqlite requests a medium file instead. //! gpt-5.6 is an unverified requested deployment, not a claim of availability. @@ -186,7 +186,7 @@ async fn manual_versa_gpt_stream_probe() { ); let model = approved_model( &required("BIOROUTER_VERSA_GPT_PROBE_MODEL"), - &required("AZURE_OPENAI_DEPLOYMENT_NAME"), + &required("VERSA_AZURE_DEPLOYMENT_NAME"), ) .unwrap_or_else(|class| panic!("{class}")); approved_endpoint(&required("AZURE_OPENAI_ENDPOINT")).unwrap_or_else(|class| panic!("{class}")); From d122b12527ce4d986cd1a2a6bcf8be74e0345353 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 10:49:10 -0700 Subject: [PATCH 2/9] fix(versa_azure): the public Azure card's endpoint and API version no longer steer Versa MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The deployment was not the only key Versa borrowed from the PUBLIC `azure_openai` card. `from_env` also fell back to its AZURE_OPENAI_ENDPOINT and AZURE_OPENAI_API_VERSION whenever the VERSA_AZURE_* twins were blank — the same population as the previous commit's. Whoever configures Azure OpenAI for a company resource changes the endpoint along with the deployment, so the previous commit alone does not repair their Versa: - Endpoint. Every Versa request posted to the company resource, carrying the transcript and VERSA_AZURE_API_KEY in its `api-key` header. The resource refused the key, and because `tier()` follows the resolved endpoint the instance became Public — a private chat could no longer bind UCSF's own model, for a reason set on a different card, and the UCSF key had gone to a host that is not UCSF's. - API version. The public card's version rode along on every request to the UCSF gateway. Reproduced on the previous commit: with the public card set to a company resource, the new routing test resolves a Versa chat to ("https://contoso.openai.azure.com", Public, "2024-10-21") instead of ("https://unified-api.ucsf.edu/general", Private, "2025-01-01-preview") The fix: Versa reads only VERSA_AZURE_ENDPOINT and VERSA_AZURE_API_VERSION, and `resolve_override` loses its legacy argument. Migration. Every Versa setup surface prefilled the endpoint with the shipped constant, unchanged since 2026-05-07, so an endpoint this drops was typed over that prefill or carried in from the public card — the bug itself. The API version is the one real migration: the onboarding card of 2026-05-30 to 2026-07-02 prefilled `2024-10-21` into AZURE_OPENAI_API_VERSION (in the Settings form the backend default, 2025-01-01-preview, won), so installs onboarded in that window and not reconnected since 2026-09-03 move from 2024-10-21 to 2025-01-01-preview — the version every other install already sends. Anyone who set either key deliberately for Versa sets its VERSA_AZURE_* twin. Privacy. This only shrinks what can move Versa's tier: the public card's endpoint could demote it, and deleting that key could promote it back; now only VERSA_AZURE_ENDPOINT can, and `tier()` still follows the endpoint the instance resolved. The key scan drops 25 -> 23 and both AZURE_OPENAI_* rows go with their reads — the reverse check added in the previous commit fails if either is left behind. azure.rs still reads all three and is not a tier-input file; azure_openai is Public wherever it points. Callers that pinned the public keys to isolate a Versa instance from the machine's config — the server's private-first-bind test and subagent_tool's UCSF child — now pin VERSA_AZURE_ENDPOINT / _API_VERSION, the keys that actually isolate it (a developer's VERSA_AZURE_ENDPOINT leaked past the old pins). The manual stream probe requires VERSA_AZURE_ENDPOINT, the Agent Drafter runbook stops saying the AZURE_OPENAI_* keys configure Versa, and privacy-tiers.md §5.1 notes that the shared-key hazard behind the demotion rule is closed at the source. --- crates/biorouter-server/src/routes/agent.rs | 4 +- crates/biorouter/src/agents/subagent_tool.rs | 2 +- crates/biorouter/src/privacy/config_keys.rs | 31 ++-- crates/biorouter/src/providers/factory.rs | 2 +- crates/biorouter/src/providers/tier_tests.rs | 5 +- crates/biorouter/src/providers/versa_azure.rs | 158 +++++++++++------- .../biorouter/tests/versa_gpt_stream_probe.rs | 4 +- .../testing/app-test-drive-runbook.md | 6 +- docs/security/privacy-tiers.md | 5 + 9 files changed, 133 insertions(+), 84 deletions(-) diff --git a/crates/biorouter-server/src/routes/agent.rs b/crates/biorouter-server/src/routes/agent.rs index 7c7e0efd4..e9f761dc9 100644 --- a/crates/biorouter-server/src/routes/agent.rs +++ b/crates/biorouter-server/src/routes/agent.rs @@ -3057,7 +3057,7 @@ mod new_session_provider_binding_tests { ); overrides.insert("VERSA_AZURE_API_KEY".into(), "test-api-key".into()); overrides.insert( - "AZURE_OPENAI_ENDPOINT".into(), + "VERSA_AZURE_ENDPOINT".into(), biorouter::providers::versa_azure::VERSA_AZURE_ENDPOINT.into(), ); overrides.insert( @@ -3069,7 +3069,7 @@ mod new_session_provider_binding_tests { .into(), ); overrides.insert( - "AZURE_OPENAI_API_VERSION".into(), + "VERSA_AZURE_API_VERSION".into(), biorouter::providers::versa_azure::VERSA_AZURE_API_VERSION.into(), ); let request = || StartAgentRequest { diff --git a/crates/biorouter/src/agents/subagent_tool.rs b/crates/biorouter/src/agents/subagent_tool.rs index 6e43afcbe..793af091c 100644 --- a/crates/biorouter/src/agents/subagent_tool.rs +++ b/crates/biorouter/src/agents/subagent_tool.rs @@ -6045,7 +6045,7 @@ mod tests { "not-a-real-key".to_string(), ), ( - "AZURE_OPENAI_ENDPOINT".to_string(), + "VERSA_AZURE_ENDPOINT".to_string(), crate::providers::versa_azure::VERSA_AZURE_ENDPOINT.to_string(), ), ]) diff --git a/crates/biorouter/src/privacy/config_keys.rs b/crates/biorouter/src/privacy/config_keys.rs index 3b48d608e..71f8c0da1 100644 --- a/crates/biorouter/src/privacy/config_keys.rs +++ b/crates/biorouter/src/privacy/config_keys.rs @@ -61,22 +61,17 @@ pub const NOT_CAPABILITY_CONFIG_KEYS: &[(&str, &str)] = &[ // 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. - ( - "AZURE_OPENAI_ENDPOINT", - "moves a Private provider's endpoint; does not raise a tier (see Task 5)", - ), - // No `AZURE_OPENAI_DEPLOYMENT_NAME` row: no tier-input file reads it. It is - // the public `azure_openai` card's required key, and `versa_azure` reading - // it as a fallback let that card's deployment route every Versa request, so - // the read was removed (2026-09-11). `azure.rs` still reads it and is not a - // tier-input file: `azure_openai` is Public whatever deployment it names. - ("AZURE_OPENAI_API_VERSION", "wire version"), - // Versa's own namespace for the same three overrides. They exist because - // onboarding used to write the `AZURE_OPENAI_*` keys above on Versa's - // behalf, which made the PUBLIC `azure_openai` card report itself - // Configured whenever a user connected UCSF's PRIVATE Versa. Same - // classification as the legacy reads they replace: they move a Private - // provider's endpoint, they do not raise a tier. + // + // 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 + // ways: onboarding WROTE them on Versa's behalf, so connecting UCSF's + // PRIVATE Versa made that PUBLIC card report itself Configured (hence this + // namespace, 2026-09-03); and Versa went on READING them as a fallback, so + // whatever that card was set up with — a company resource's endpoint, + // deployment and API version — steered every Versa request (read removed + // 2026-09-11). No tier-input file reads the `AZURE_OPENAI_*` keys now, so + // they have no rows here; `azure.rs` still reads them and is not a + // 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)", @@ -187,10 +182,10 @@ mod tests { // of the two lists. Adding a config read to any of them fails this test // until someone decides whether it determines capability. That is the // checkable list: it does not depend on anyone remembering a rule. - let scanned = scan_get_param_keys(); // 25 today + let scanned = scan_get_param_keys(); // 23 today assert_eq!( scanned.len(), - 25, + 23, "the tier-input files' config surface changed: {scanned:?}" ); for key in &scanned { diff --git a/crates/biorouter/src/providers/factory.rs b/crates/biorouter/src/providers/factory.rs index d872c9072..79eea02a9 100644 --- a/crates/biorouter/src/providers/factory.rs +++ b/crates/biorouter/src/providers/factory.rs @@ -510,7 +510,7 @@ pub(crate) mod tests { ), ( "versa_azure", - "Institution(ucsf): `ucsf_gateway_affiliation` on the resolved AZURE_OPENAI_ENDPOINT", + "Institution(ucsf): `ucsf_gateway_affiliation` on the resolved VERSA_AZURE_ENDPOINT", ), ]; #[cfg(feature = "aws-providers")] diff --git a/crates/biorouter/src/providers/tier_tests.rs b/crates/biorouter/src/providers/tier_tests.rs index 077b64bc4..ebab8c16e 100644 --- a/crates/biorouter/src/providers/tier_tests.rs +++ b/crates/biorouter/src/providers/tier_tests.rs @@ -291,8 +291,9 @@ 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 reads AZURE_OPENAI_ENDPOINT, the same key the public - // azure_openai provider reads, and versa_bedrock falls back to + // 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. diff --git a/crates/biorouter/src/providers/versa_azure.rs b/crates/biorouter/src/providers/versa_azure.rs index bb5893674..7c681196d 100644 --- a/crates/biorouter/src/providers/versa_azure.rs +++ b/crates/biorouter/src/providers/versa_azure.rs @@ -182,8 +182,8 @@ pub struct VersaAzureProvider { model: ModelConfig, name: String, /// The endpoint this instance resolved at construction. `tier()` reads it, - /// never the provider's name — the three `AZURE_OPENAI_*` keys are shared - /// with the public `azure_openai` provider and are user-writable. + /// never the provider's name — `VERSA_AZURE_ENDPOINT` is user-writable, so + /// an instance can resolve somewhere that is not the UCSF gateway. resolved_endpoint: String, credential_source: VersaAzureCredentialSource, } @@ -229,18 +229,13 @@ impl AuthProvider for VersaAzureAuthProvider { } } -/// Resolve one endpoint override: this provider's own key first, the public -/// Azure provider's key second, the shipped UCSF default last. +/// Resolve one override: this provider's own key, else the shipped UCSF +/// default. There is no third source — see `from_env` for the one there was. /// -/// Pure, and split out from `from_env`, because the ORDER is the whole fix and -/// a closure inside a function that reads global config cannot be tested. /// A blank stored value is treated as absent -- writing an empty string into /// the box in Advanced means "use the default", not "point at nowhere". -fn resolve_override(fallback: &str, own: Option, legacy: Option) -> String { - [own, legacy] - .into_iter() - .flatten() - .find(|value| !value.trim().is_empty()) +fn resolve_override(fallback: &str, own: Option) -> String { + own.filter(|value| !value.trim().is_empty()) .unwrap_or_else(|| fallback.to_string()) } @@ -248,30 +243,40 @@ impl VersaAzureProvider { pub async fn from_env(model: ModelConfig) -> Result { let config = crate::config::Config::global(); - // Overrides are read from this provider's OWN namespace first, and only - // then from the public Azure provider's keys. + // Overrides are read from this provider's OWN namespace, and nowhere + // else. // - // ⚠ The order is the fix, not a preference. `AZURE_OPENAI_ENDPOINT` and - // its two siblings belong to the `azure_openai` card, and - // `check_provider_configured` reads exactly those to decide whether that - // card says Configured. While onboarding wrote them on Versa's behalf, + // ⚠ `AZURE_OPENAI_ENDPOINT` and its two siblings belong to the public + // `azure_openai` card — `check_provider_configured` reads them to decide + // whether that card says Configured — and sharing them went wrong in + // both directions. Onboarding WROTE them on Versa's behalf, so // connecting UCSF's PRIVATE Versa lit up the PUBLIC Azure OpenAI card as - // configured -- a provider the user never set up, sitting one click away - // in the same grid, and Public where Versa is Private. The legacy read - // stays so an install that customised those keys before this change - // keeps resolving to the same endpoint. + // configured; that is why these `VERSA_AZURE_*` keys exist + // (2026-09-03). And Versa went on READING them as a fallback, so + // whatever that card was set up with steered Versa: a company Azure + // resource's endpoint received every Versa request — the transcript, + // with `VERSA_AZURE_API_KEY` in its `api-key` header — refused the key, + // and the instance turned Public. Its deployment and API version rode + // along. The fallback is gone (2026-09-11). + // + // Every Versa setup form prefilled the endpoint constant, so an endpoint + // this drops was either typed over the prefill or carried in from the + // public card — the bug itself. The API version is the exception: the + // onboarding card of 2026-05-30 to 2026-07-02 prefilled `2024-10-21` + // into the legacy key, and those installs now send the shipped version + // every other install sends. + // // ⚠ Every key below is a STRING LITERAL passed straight to `get_param`, // and it has to stay that way. `privacy::config_keys` scans this file for // literal-keyed `get_param` calls to build the list of keys that move a // provider's tier, and a key assembled at runtime — even one as innocent - // as a `|own, legacy|` closure parameter — is invisible to that scan. The - // first draft of this fix did exactly that and took the three - // `AZURE_OPENAI_*` keys off the privacy surface without anyone deciding - // to; two tests in that module caught it. + // as an `|own, legacy|` closure parameter — is invisible to that scan. + // The first draft of the 2026-09-03 namespacing did exactly that and + // took three keys off the privacy surface without anyone deciding to; + // two tests in that module caught it. let endpoint = resolve_override( VERSA_AZURE_ENDPOINT, config.get_param::("VERSA_AZURE_ENDPOINT").ok(), - config.get_param::("AZURE_OPENAI_ENDPOINT").ok(), ); // There is no shipped default deployment any more: the model picks it. // What the configuration names is a CANDIDATE override, and @@ -310,7 +315,6 @@ impl VersaAzureProvider { let api_version = resolve_override( VERSA_AZURE_API_VERSION, config.get_param::("VERSA_AZURE_API_VERSION").ok(), - config.get_param::("AZURE_OPENAI_API_VERSION").ok(), ); // ⚠ `.ok()` here used to discard the REASON the key was unavailable, and @@ -664,27 +668,16 @@ impl Provider for VersaAzureProvider { mod tests { use super::*; + /// That the public card's keys are never read is asserted where it can be + /// seen — on a real `from_env` in `routing_tests` — not here. #[test] - fn an_override_prefers_versas_own_key_then_the_legacy_one_then_the_default() { - // Own key wins. - assert_eq!( - resolve_override("default", Some("own".into()), Some("legacy".into())), - "own" - ); - // An install that customised the shared key BEFORE the namespace existed - // still resolves to the endpoint it chose. - assert_eq!( - resolve_override("default", None, Some("legacy".into())), - "legacy" - ); + fn an_override_is_versas_own_key_or_the_default() { + assert_eq!(resolve_override("default", Some("own".into())), "own"); // Nothing stored: the shipped UCSF default, which is why onboarding never // needed to write these keys at all. - assert_eq!(resolve_override("default", None, None), "default"); + assert_eq!(resolve_override("default", None), "default"); // Blank is absent, not "point at nowhere". - assert_eq!( - resolve_override("default", Some(" ".into()), None), - "default" - ); + assert_eq!(resolve_override("default", Some(" ".into())), "default"); } use crate::providers::api_client::AuthMethod; @@ -723,10 +716,9 @@ mod tests { /// this provider calls it, or hands it the right field. Replace the body of /// `tier()` with an unconditional `Private`, or point it at a field that is /// not the resolved endpoint, and every one of those tests still passes. - /// This one does not: the three `AZURE_OPENAI_*` keys are shared with the - /// public `azure_openai` provider, so a `tier()` that ignores the endpoint - /// hands a private badge to a provider posting transcripts wherever the - /// user's config points. + /// This one does not: `VERSA_AZURE_ENDPOINT` is user-writable config, so a + /// `tier()` that ignores the endpoint hands a private badge to a provider + /// posting transcripts wherever that config points. #[test] fn tier_follows_the_endpoint_this_instance_resolved() { let shipped = test_provider(); @@ -753,8 +745,8 @@ mod tests { /// it or hands it the right field. Returning an unconditional /// `Institution("ucsf")` here — or keying it on `get_name()`, which is the /// obvious implementation — passes every one of those tests and hands a UCSF - /// badge to an instance posting prompts wherever the user's shared - /// `AZURE_OPENAI_ENDPOINT` points. + /// badge to an instance posting prompts wherever the user's + /// `VERSA_AZURE_ENDPOINT` points. #[test] fn affiliation_follows_the_endpoint_this_instance_resolved() { use crate::privacy::affiliation::{InstitutionId, ModelAffiliation}; @@ -1239,6 +1231,23 @@ mod routing_tests { .collect() } + /// The `api-version` each request carried, in order. + async fn requested_api_versions(server: &MockServer) -> Vec { + server + .received_requests() + .await + .unwrap_or_default() + .iter() + .filter_map(|request| { + request + .url + .query_pairs() + .find(|(name, _)| name == "api-version") + .map(|(_, version)| version.into_owned()) + }) + .collect() + } + fn path_of(deployment: &str) -> String { format!("/openai/deployments/{deployment}/chat/completions") } @@ -1255,17 +1264,16 @@ mod routing_tests { provider } - /// A key, Versa's deployment key and the public `azure_openai` card's as - /// given, and every other override key blank — blank is absent — so the - /// machine running the suite cannot leak its own configuration into what is - /// being measured. + /// A key, Versa's deployment key as given, and its other two override keys + /// blank — blank is absent — so the machine running the suite cannot leak + /// its own configuration into what is being measured. Plus the public + /// `azure_openai` card's deployment key as given, which `from_env` must not + /// read; the tests that set it are how that is known. fn config(deployment: &str, public_deployment: &str) -> HashMap { HashMap::from([ ("VERSA_AZURE_API_KEY".into(), "test-key".into()), ("VERSA_AZURE_ENDPOINT".into(), String::new()), - ("AZURE_OPENAI_ENDPOINT".into(), String::new()), ("VERSA_AZURE_API_VERSION".into(), String::new()), - ("AZURE_OPENAI_API_VERSION".into(), String::new()), ("VERSA_AZURE_DEPLOYMENT_NAME".into(), deployment.into()), ( "AZURE_OPENAI_DEPLOYMENT_NAME".into(), @@ -1483,6 +1491,44 @@ mod routing_tests { ); } + /// The public card's other two keys. Wherever `VERSA_AZURE_ENDPOINT` was + /// blank, its endpoint used to become Versa's, so a company's Azure + /// resource received every Versa request — the transcript, with + /// `VERSA_AZURE_API_KEY` in its `api-key` header — refused the key, and the + /// instance turned Public. Its API version rode along on every request to + /// the gateway. + #[tokio::test] + async fn the_public_azure_cards_endpoint_and_api_version_never_reach_versa() { + let server = gateway().await; + let mut public_card = config("", "my-gpt4o"); + public_card.insert( + "AZURE_OPENAI_ENDPOINT".into(), + "https://contoso.openai.azure.com".into(), + ); + // Azure's GA version — and what Versa's own onboarding card wrote into + // this key from 2026-05-30 to 2026-07-02, which is why it is this one. + public_card.insert("AZURE_OPENAI_API_VERSION".into(), "2024-10-21".into()); + + let chat = bound("gpt-4.1-2025-04-14", public_card).await; + let binding = serde_json::to_value(chat.restore_binding()).unwrap(); + assert_eq!( + (&binding["endpoint"], chat.tier(), &binding["api_version"]), + ( + &serde_json::json!(VERSA_AZURE_ENDPOINT), + ProviderTier::Private, + &serde_json::json!(VERSA_AZURE_API_VERSION), + ), + "the public azure_openai card's endpoint or API version reached a Versa chat" + ); + + // And on the wire: the version the gateway was actually sent. + answered_by(&aimed_at(chat, &server)).await.unwrap(); + assert_eq!( + requested_api_versions(&server).await, + vec![VERSA_AZURE_API_VERSION] + ); + } + /// Every row written before this change stores gpt-5.5's deployment, /// whatever its model — the QA run read exactly that off a rebound chat. A /// restore must re-derive the route from the model, not carry it forward. diff --git a/crates/biorouter/tests/versa_gpt_stream_probe.rs b/crates/biorouter/tests/versa_gpt_stream_probe.rs index 9d4b9116d..c2297ed17 100644 --- a/crates/biorouter/tests/versa_gpt_stream_probe.rs +++ b/crates/biorouter/tests/versa_gpt_stream_probe.rs @@ -1,6 +1,6 @@ //! Opt-in synthetic provider/decoder probe; returned tools are never executed. //! Requires BIOROUTER_RUN_VERSA_GPT_PROBE=1, BIOROUTER_VERSA_GPT_PROBE_MODEL, -//! matching VERSA_AZURE_DEPLOYMENT_NAME, AZURE_OPENAI_ENDPOINT, and a NEW +//! matching VERSA_AZURE_DEPLOYMENT_NAME, VERSA_AZURE_ENDPOINT, and a NEW //! BIOROUTER_VERSA_GPT_PROBE_OUTPUT directly under /tmp. CASE defaults to todo; //! BIOROUTER_VERSA_GPT_PROBE_CASE=python_sqlite requests a medium file instead. //! gpt-5.6 is an unverified requested deployment, not a claim of availability. @@ -189,7 +189,7 @@ async fn manual_versa_gpt_stream_probe() { &required("VERSA_AZURE_DEPLOYMENT_NAME"), ) .unwrap_or_else(|class| panic!("{class}")); - approved_endpoint(&required("AZURE_OPENAI_ENDPOINT")).unwrap_or_else(|class| panic!("{class}")); + approved_endpoint(&required("VERSA_AZURE_ENDPOINT")).unwrap_or_else(|class| panic!("{class}")); let case = std::env::var("BIOROUTER_VERSA_GPT_PROBE_CASE").unwrap_or_else(|_| "todo".into()); synthetic_case(&case).unwrap_or_else(|class| panic!("{class}")); let path = required("BIOROUTER_VERSA_GPT_PROBE_OUTPUT"); diff --git a/docs/agent-drafter/testing/app-test-drive-runbook.md b/docs/agent-drafter/testing/app-test-drive-runbook.md index db4db59c4..045a9873f 100644 --- a/docs/agent-drafter/testing/app-test-drive-runbook.md +++ b/docs/agent-drafter/testing/app-test-drive-runbook.md @@ -117,8 +117,10 @@ print('wrote', envf, '(len', len(key), ')') PY ``` -The provider env vars (`AZURE_OPENAI_ENDPOINT`, deployment, api-version) are already in -`~/.config/biorouter/config.yaml`; only the key needs supplying. +Only the key needs supplying: `versa_azure`'s endpoint, deployment and API version are compiled +in, and `VERSA_AZURE_ENDPOINT` / `_DEPLOYMENT_NAME` / `_API_VERSION` in +`~/.config/biorouter/config.yaml` override them. It does not read the public Azure provider's +`AZURE_OPENAI_*` keys, so those change nothing here. ### 1.5 Start the daemon diff --git a/docs/security/privacy-tiers.md b/docs/security/privacy-tiers.md index 8153afa87..dc3e9437a 100644 --- a/docs/security/privacy-tiers.md +++ b/docs/security/privacy-tiers.md @@ -677,6 +677,11 @@ reads `AZURE_OPENAI_ENDPOINT` / `AZURE_OPENAI_DEPLOYMENT_NAME` / `AZURE_OPENAI_A compiled-in UCSF gateway (`unified-api.ucsf.edu`), computed at construction when the endpoint is already resolved. +> **Update (2026-09-11).** `versa_azure` now reads only its own `VERSA_AZURE_ENDPOINT` / +> `_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. + **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 construction. From bb988b644ee6255a06365bf4d93697b82a4d6fe5 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 11:53:55 -0700 Subject: [PATCH 3/9] test(bedrock): reproduce the public/private Bedrock namespace crossing These rows FAIL at this commit, on purpose: they assert the routing the next commits establish, and this is the code they were run against. Versa Bedrock (Private, UCSF) and the public Amazon Bedrock card shared the AWS_* namespace, and it went wrong in both directions: - The public card's AWS_REGION, and an AWS_ENDPOINT_URL_BEDROCK in config or the process environment, became Versa's region and endpoint. The instance turned Public and signed with UCSF-issued keys for someone's AWS region. - AWS_BEARER_TOKEN_BEDROCK in the environment (the AWS SDK's own variable for a Bedrock API key) made the SDK authenticate Versa's requests with that bearer token instead of signing them, so 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. - The AWS_ENDPOINT_URL_BEDROCK Versa's setup persisted became the PUBLIC provider's endpoint once bedrock.rs exported and promoted it, so the user's own AWS-signed requests went to UCSF's gateway. - Versa declared AWS_REGION, which the public card declares too, so a Versa setup marked that card Configured; the onboarding card wrote both AWS keys. Each provider is built through from_env and only its HTTP transport is then swapped for the SDK's capture client, so every assertion is on the request production would have sent, and nothing leaves the process. Environment rows run in a re-executed child process that starts with the scenario's environment and its own config root, because set_var in the test binary is unsound and the public provider's own set_var is part of what is measured. --- crates/biorouter/src/providers/bedrock.rs | 13 + .../src/providers/bedrock_namespace_tests.rs | 544 ++++++++++++++++++ crates/biorouter/src/providers/mod.rs | 2 + .../biorouter/src/providers/versa_bedrock.rs | 14 + .../InstitutionalSetupCard.test.tsx | 30 + 5 files changed, 603 insertions(+) create mode 100644 crates/biorouter/src/providers/bedrock_namespace_tests.rs diff --git a/crates/biorouter/src/providers/bedrock.rs b/crates/biorouter/src/providers/bedrock.rs index 3bfbed5d8..6e68e54ce 100644 --- a/crates/biorouter/src/providers/bedrock.rs +++ b/crates/biorouter/src/providers/bedrock.rs @@ -137,6 +137,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..3316f5d61 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; diff --git a/crates/biorouter/src/providers/versa_bedrock.rs b/crates/biorouter/src/providers/versa_bedrock.rs index 3fbf003d9..c69190ab8 100644 --- a/crates/biorouter/src/providers/versa_bedrock.rs +++ b/crates/biorouter/src/providers/versa_bedrock.rs @@ -238,6 +238,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") diff --git a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx index e473575fa..cc59df489 100644 --- a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx +++ b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx @@ -27,6 +27,15 @@ async function connectVersaAzure() { await waitFor(() => expect(mockCheckProvider).toHaveBeenCalled()); } +async function connectVersaBedrock() { + 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 })); + await waitFor(() => expect(mockCheckProvider).toHaveBeenCalled()); +} + describe('InstitutionalSetupCard', () => { beforeEach(() => { vi.clearAllMocks(); @@ -55,4 +64,25 @@ 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 its namespaced overrides', async () => { + await connectVersaBedrock(); + const written = mockUpsert.mock.calls.map((c) => c[0] as string); + expect(written).toContain('VERSA_BEDROCK_ACCESS_KEY_ID'); + expect(written).toContain('VERSA_BEDROCK_SECRET_ACCESS_KEY'); + expect(written).toContain('VERSA_BEDROCK_ENDPOINT'); + expect(written).toContain('VERSA_BEDROCK_REGION'); + }); }); From 038b48187515f3d967e66afa25fe8e0d524a3f6f Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 11:58:02 -0700 Subject: [PATCH 4/9] fix(versa_bedrock): read the endpoint and region from Versa's own namespace Versa Bedrock read the public Amazon Bedrock card's AWS_REGION and an AWS_ENDPOINT_URL_BEDROCK key, then fell back to AWS_ENDPOINT_URL_BEDROCK and AWS_ENDPOINT_URL_BEDROCK_RUNTIME in the process environment. bedrock.rs exports every AWS_* config value into that environment, and a shell holds them for the AWS CLI, so whatever the public side was set up with steered Versa: UCSF-issued keys signed requests for someone's own AWS region, which refused them, and the instance turned Public. It reads VERSA_BEDROCK_ENDPOINT and VERSA_BEDROCK_REGION now, and nothing else; blank is absent, absent is the UCSF gateway and us-west-2. It also stops declaring the two AWS keys. AWS_REGION is one of the two keys the public card declares, both defaulted, so check_provider_configured called that card Configured as soon as a Versa setup persisted it. Privacy key scan (privacy/config_keys.rs): the scanned count stays 23, with two rows swapped rather than added. AWS_ENDPOINT_URL_BEDROCK and AWS_REGION are deleted, since no tier-input file reads them now, and the reverse check insists on that. VERSA_BEDROCK_ENDPOINT keeps its predecessor's classification, NOT a capability key. The stated reason was false: tier() has followed the resolved endpoint since e2e4eb9d, so deleting an off-site value does raise the tier. What holds instead is that only the UCSF gateway's own host reads Private, so no write can forge the badge. Whether that raise should still be a user act is left as an open DR-16 question in the comment. VERSA_BEDROCK_REGION is the SigV4 signing region; the endpoint, not the region, decides where a request goes. The manual versa_stream_wire_probe mirrors from_env and reads the same keys. At this commit the environment rows still fail, on the bearer token alone: renaming the keys does not stop the AWS SDK reading AWS_BEARER_TOKEN_BEDROCK. That is the next commit. --- crates/biorouter/src/privacy/config_keys.rs | 37 +++++--- .../biorouter/src/providers/versa_bedrock.rs | 95 ++++++++++--------- .../tests/versa_stream_wire_probe.rs | 7 +- 3 files changed, 81 insertions(+), 58 deletions(-) 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/versa_bedrock.rs b/crates/biorouter/src/providers/versa_bedrock.rs index c69190ab8..ffd383763 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); @@ -381,21 +389,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() @@ -766,10 +777,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; @@ -787,12 +797,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 From 1bf44605bf7c29d56d6d36fc7cb21a4922df2f20 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 12:00:03 -0700 Subject: [PATCH 5/9] fix(versa_bedrock): sign with Versa's own keys even beside a Bedrock API key The AWS SDK reads AWS_BEARER_TOKEN_BEDROCK from the process environment by itself. 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. So a Versa chat that looked entirely right, with the UCSF gateway, us-west-2 and a Private tier, sent the public card's API key to UCSF in its Authorization header, and Versa's own keys signed nothing. Measured through the real from_env with the SDK's capture client: authorization: "Bearer public-bedrock-api-key" (host unified-api.ucsf.edu) Versa's loader now prefers SigV4 explicitly. A preference set on the loader counts as chosen in code (Origin::is_client_config), so the SDK's env branch leaves it alone, and the request is signed with VERSA_BEDROCK_* for the gateway's region. from_resolved is the one constructor, so a restored chat gets the same pin. The endpoint was never exposed this way, and that was checked rather than assumed: an endpoint set on the loader is also chosen in code, so AWS_ENDPOINT_URL_BEDROCK_RUNTIME in the environment does not re-aim the client. nothing_in_the_process_environment_steers_versa pins both. --- crates/biorouter/src/providers/versa_bedrock.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/crates/biorouter/src/providers/versa_bedrock.rs b/crates/biorouter/src/providers/versa_bedrock.rs index ffd383763..0a78d9c76 100644 --- a/crates/biorouter/src/providers/versa_bedrock.rs +++ b/crates/biorouter/src/providers/versa_bedrock.rs @@ -206,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)] From f9f16e787ff9abd807511886ad98b6ceb12d1ee7 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 12:02:27 -0700 Subject: [PATCH 6/9] fix(bedrock): the endpoint Versa's setup persisted no longer routes the public provider Versa Bedrock's setup persisted AWS_ENDPOINT_URL_BEDROCK pointing at UCSF's gateway: its onboarding card on every connect, its Settings form on every save. bedrock.rs exports every AWS_* config value into the process environment and then promoted AWS_ENDPOINT_URL_BEDROCK to AWS_ENDPOINT_URL_BEDROCK_RUNTIME, the variable the AWS SDK aims Bedrock Runtime at. So once the PUBLIC Amazon Bedrock provider was built on an install that had set up Versa, the user's own AWS-signed requests went to UCSF's gateway: host: "unified-api.ucsf.edu" authorization: "AWS4-HMAC-SHA256 Credential=PUBLICTESTACCESSKEY/.../us-west-2/bedrock/..." The promotion is gone. Commit ac7def8c added it on 2026-04-12, a month before Versa Bedrock existed, for "configs and setup scripts" that used the short name. Every Biorouter surface that has written the key since wrote it for Versa. The previous commits stop new writes, but installs keep the key, so this provider has to ignore it, not merely stop receiving it. The short name is also not Bedrock Runtime's: the SDK derives AWS_ENDPOINT_URL_BEDROCK for the Bedrock control plane. The public provider still follows the SDK's own mechanisms, which is how a VPC endpoint or a proxy is meant to be set: AWS_ENDPOINT_URL_BEDROCK_RUNTIME in the environment, the same key in config.yaml via the export, or an AWS profile's services section. the_public_provider_still_follows_the_sdks_ endpoint_variable is the positive control, and it passes before and after. This also removes one of the unlocked read-then-write set_var sites that docs/testing/process-global-state.md lists. --- crates/biorouter/src/providers/bedrock.rs | 26 +++++++++++++++-------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/crates/biorouter/src/providers/bedrock.rs b/crates/biorouter/src/providers/bedrock.rs index 6e68e54ce..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()); From 981b88fb265aa1414291e2b5067a36f50005ee0e Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 12:04:16 -0700 Subject: [PATCH 7/9] fix(desktop): connecting Versa Bedrock no longer writes the public AWS keys The onboarding card upserted AWS_ENDPOINT_URL_BEDROCK and AWS_REGION on every Versa Bedrock connect, with the prefilled UCSF gateway and us-west-2, from an Advanced panel that also offered them for editing. Both belong to the public Amazon Bedrock card: AWS_REGION marked that card Configured and replaced its region, and the public provider took the gateway as its endpoint. The card now writes VERSA_BEDROCK_ENDPOINT and VERSA_BEDROCK_REGION, the only keys versa_bedrock.rs reads, and labels the Advanced fields with those names. The Settings form's versa_bedrock entry in PROVIDER_KEY_DEFAULTS is dead now: a default only reaches a key the provider declares, and versa_bedrock declares its key and secret alone. It is deleted rather than renamed. Persisted values are not touched; the provider no longer reads them. --- .../onboarding/InstitutionalSetupCard.tsx | 22 ++++++++++++------- .../forms/DefaultProviderSetupForm.tsx | 4 ---- 2 files changed, 14 insertions(+), 12 deletions(-) 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) => { From 35f69b2aaf291731291d88e9a6a650caed733aa4 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 12:06:26 -0700 Subject: [PATCH 8/9] docs: the shared-namespace hazard is closed for Versa Bedrock too - privacy-tiers.md 5.1: a dated Bedrock note beside the Azure one. It says what closed, and names the crossing no endpoint check could see: the SDK's own read of AWS_BEARER_TOKEN_BEDROCK put the public card's API key on Versa requests to the gateway. The demotion rule stays, because VERSA_BEDROCK_ENDPOINT is user-writable. - process-global-state.md: the promotion at bedrock.rs:90-92 leaves the table of production env writers (five sites become four, with a note saying what the fifth did), and the ledger gains a row for AWS_BEARER_TOKEN_BEDROCK. That reader lives inside a dependency, which is why no env::var scan in the document could have found it. - providers/mod.rs and tier_tests.rs: the reason given for the demotion named both shared-key hazards as current. The reason that holds now is that each Versa endpoint is user-writable config. --- crates/biorouter/src/providers/mod.rs | 16 +++++++--------- crates/biorouter/src/providers/tier_tests.rs | 11 +++++------ docs/security/privacy-tiers.md | 10 ++++++++++ docs/testing/process-global-state.md | 6 ++++-- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/crates/biorouter/src/providers/mod.rs b/crates/biorouter/src/providers/mod.rs index 3316f5d61..f978c5aea 100644 --- a/crates/biorouter/src/providers/mod.rs +++ b/crates/biorouter/src/providers/mod.rs @@ -134,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 @@ -166,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/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 From 50a55ed4d6c4feabf52d216e6db183d40653f050 Mon Sep 17 00:00:00 2001 From: Wanjun Gu Date: Fri, 11 Sep 2026 12:11:42 -0700 Subject: [PATCH 9/9] test(desktop): the Versa Bedrock card tests wait for the whole connect The helper waited for checkProvider, so a write the card makes after it, such as BIOROUTER_PROVIDER, landed after the assertions ran. A late AWS_* write would have slipped past the rule the first test states. It now waits for onSuccess('versa_bedrock'), as #234 does for the Azure flavour, and the second test pins the exact write sequence: key, secret, the two VERSA_BEDROCK_* overrides with the shipped defaults, then the provider. Run against the pre-fix card (778381d4), both fail, and the diff shows what it wrote: the same shipped defaults, under AWS_ENDPOINT_URL_BEDROCK and AWS_REGION. --- .../InstitutionalSetupCard.test.tsx | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx index cc59df489..847a9befc 100644 --- a/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx +++ b/ui/desktop/src/components/onboarding/InstitutionalSetupCard.test.tsx @@ -28,12 +28,14 @@ async function connectVersaAzure() { } async function connectVersaBedrock() { - render(); + 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 })); - await waitFor(() => expect(mockCheckProvider).toHaveBeenCalled()); + // Past `checkProvider`, so every write the connect makes has been recorded. + await waitFor(() => expect(onSuccess).toHaveBeenCalledWith('versa_bedrock')); } describe('InstitutionalSetupCard', () => { @@ -77,12 +79,14 @@ describe('InstitutionalSetupCard', () => { expect(written.filter((key) => key.startsWith('AWS_'))).toEqual([]); }); - it('writes the Versa Bedrock credentials and its namespaced overrides', async () => { + it('writes the Versa Bedrock credentials and overrides, then selects the provider', async () => { await connectVersaBedrock(); - const written = mockUpsert.mock.calls.map((c) => c[0] as string); - expect(written).toContain('VERSA_BEDROCK_ACCESS_KEY_ID'); - expect(written).toContain('VERSA_BEDROCK_SECRET_ACCESS_KEY'); - expect(written).toContain('VERSA_BEDROCK_ENDPOINT'); - expect(written).toContain('VERSA_BEDROCK_REGION'); + 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], + ]); }); });